Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rich text redux #2213

Merged
merged 44 commits into from
Dec 12, 2021
Merged

Rich text redux #2213

merged 44 commits into from
Dec 12, 2021

Conversation

Efruit
Copy link
Contributor

@Efruit Efruit commented Nov 9, 2021

The preparation for Books and #2101 begins.

@Acruid Acruid added the Status: Stale The PR has gone more than 14 days without activity. label Nov 24, 2021
@Acruid Acruid removed the Status: Stale The PR has gone more than 14 days without activity. label Nov 25, 2021
@Efruit Efruit mentioned this pull request Nov 27, 2021
Open
@Efruit
Copy link
Contributor Author

Efruit commented Nov 27, 2021

The content side is on my fork's branch of the same name. Not gonna bother PRing it (yet) since this one still needs to be torn to shreds first.

@Efruit Efruit marked this pull request as ready for review November 30, 2021 19:43
@Efruit Efruit mentioned this pull request Nov 30, 2021
Merged
@Efruit
Copy link
Contributor Author

Efruit commented Nov 30, 2021

Content side at space-wizards/space-station-14#5625

@Efruit
Copy link
Contributor Author

Efruit commented Nov 30, 2021 •
edited
Loading

Future Improvements:

  • Section.Meta flag for color not being set
  • ISectionable that runs Loc.GetString on sections with Section.Meta's Localized flag (requires extra metadata for fluent args)
  • Clickable links (also requires extra metadata)
  • Reduce the number of FormattedMessage.Builder call sites (constructing Sections really is NOT hard)

@Acruid Acruid added Status: Requires Content PR This PR breaks content and requires both to be merged together. Issue: Feature This issue is a feature request labels Dec 9, 2021
Acruid
Acruid suggested changes Dec 9, 2021
View reviewed changes
Copy link
Contributor

@Acruid Acruid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the obsolete 'FormattedMessage.Builder' warnings need to be fixed.

Robust.Shared/Utility/CollectionExtensions.cs
@@ -6,7 +6,7 @@

namespace Robust.Shared.Utility
{
public static class Extensions
public static partial class Extensions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this turned into a partial class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strictly for personal reasons

(Because having Robust.Shared.Utility.Extensions and Robust.Shared.Utility.TextExtensions seemed messy, and I didn't want to rename the existing Extensions to CollectionExtensions.)

@Efruit
Copy link
Contributor Author

Efruit commented Dec 9, 2021

All of the obsolete 'FormattedMessage.Builder' warnings need to be fixed.

It'll have to be an ongoing process. Builder was only added as a stopgap to make the transition easier. It's a warning because it should NOT be used in new code. After Content has caught up, and no longer needs it, we can make it internal and nix the warning.

@Efruit
Copy link
Contributor Author

Efruit commented Dec 10, 2021

To expand a bit on that, Builder is not, in fact, obsolete. It has sanctioned (engine-internal) uses within markup parsers, since they need something that does exactly what Builder does.

Content, on the other hand, should either be putting Sections together manually (it's not hard), or using a markup parser.

But I haven't been able to find the [PleaseDontUseThisExceptInThesePlaces] attribute, so I went with [Obsolete] instead.

@Efruit Efruit mentioned this pull request Dec 11, 2021
Open
@Efruit
UnitTesting: Update FormattedMessage/Markup Tests
90f042a 
They will NOT pass yet, but I don't care; it compiles.
@Efruit
Utility/FormattedMessage: Fix some off-by-one Builder bugs
3881489
@Efruit
Utility/FormattedMessage: Continue cleanup, test compliance
9accc73
@Efruit
Utility/FormattedMessage: Work around dotnet/roslyn#57870
1d31ebd
@Efruit
Utility/FormattedMessage: Move ISectionable from TextLayout, implemen…
b5dfd28 
…t it for FormattedMessage
@Efruit
UserInterface/TextLayout: Add a postcreate function to set up new `…
5caa72f 
…TIn`s

Apparently Roslyn isn't big-brained enough to understand that a
closure of type `Func<T>` means that `var n = new(); return n;` requires
`new()` to return a `T`.

Ironically, it's significantly less cbt to add this than to convert
that big tuple in `Layout` in to a class or struct of some sort
(to initialize the `List<>`s).
@Efruit
UserInterface/TextLayout: Throw if Meta isn't recognized
e3b154e 
TODO warning go brrr
@Efruit
Graphics/FontLibrary: Add a DummyVariant
a3f9e07
@Efruit
UserInterface/UITheme: Move to FontLibraries
e8ed541
@Efruit
UserInterface/TextLayout: Move to an un-nested ImmutableArray
384cd4f
@Efruit
UserInterface/RichTextEntry: Go ahead. Draw.
a6f0f45
@Efruit
Markup/Basic: Add extension & helpers for FormattedMessage.Builder
f47cf6e
@Efruit
Markup/Basic: Add EscapeText back in
d8238a6 
A forgotten casualty of the great Markup separation of 2021
@Efruit
Graphics/FontLibrary: Clean up bit magic, ensure that at least one fo…
b99acfc 
…nt is picked
@Efruit
Graphics/FontLibrary: Add diagnostics to the "no fonts" exception
5b685b6
@Efruit
UserInterface/TextLayout: Scrap Word, return to Offset
f8ab2b2
@Efruit
UserInterface/TextLayout: A whole bunch of hard-fought bugfixes
bbbf031
@Efruit
Utility/FormattedMessage: Add a static, empty FormattedMessage
a1ffdf5
@Efruit
Utility/FormattedMessage: Fix. Bugs.
7f7876c
@Efruit
UserInterface/RichTextEntry: Bug fixin'
48951a3
@Efruit
UserInterface: CSS teim
569ce41
@Efruit
Markup/Basic: Add an optional "default" style to use
b2f02ce
@Efruit
Utility/FormattedMessage: I'm surprised I only made this mistake once.
0d07f08
@Efruit
Log/DebugConsoleLogHandler: work around lack of a default style
c2ec30f
@Acruid Acruid merged commit 2a8887d into space-wizards:master Dec 12, 2021
@Efruit Efruit mentioned this pull request Dec 19, 2021
Closed
PaulRitter added a commit to PaulRitter/RobustToolbox that referenced this pull request Dec 20, 2021
@PaulRitter
Revert "Rich text redux (space-wizards#2213)"
b6f623a 
This reverts commit 2a8887d.
@PaulRitter PaulRitter added the Status: Derelict This PR was closed but might contain useful code that can be salvaged. label Dec 20, 2021
@ElectroJr ElectroJr removed the Status: Derelict This PR was closed but might contain useful code that can be salvaged. label Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Acruid Acruid Acruid approved these changes

@PJB3005 PJB3005 Awaiting requested review from PJB3005 PJB3005 is a code owner

@ZoldorfTheWizard ZoldorfTheWizard Awaiting requested review from ZoldorfTheWizard

Assignees
No one assigned
Labels
Issue: Feature This issue is a feature request Status: Requires Content PR This PR breaks content and requires both to be merged together.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Efruit @Acruid @PaulRitter @ElectroJr