-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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.
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
StyleNano
removal: Palette system and Sheetlets
#29903
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
… methods to `ButtonSheetlet` Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
…correctly Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
…ses` syntax is dead Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
…ormat Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
Signed-off-by: Brandon Li <[email protected]>
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
b6b7072
to
cd8f1cb
Compare
cd8f1cb
to
b6b7072
Compare
Sorry to the two people I just requested a review from, just a side effect of my epic git skills. Okay, I may need some help to help give Moony commmit credit. I'm decently familiar with git, but I'm not a wizard and this is tricky. I tried checking out the commit in question, amending it with Not sure what to do now, that was the one idea I had. |
I'll resolve the merge conflicts once someone gives me the go ahead because insert Sisyphus image Also, to be clear, this PR does NOT change maps or sprites. again, side effect of my epic git skills. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't do a whole review but this should be the core of the actual system + half of the rest.
public const string LabelKeyText = "LabelKeyText"; | ||
public const string LabelWeak = "LabelWeak"; // replaces `StyleClassLabelSecondaryColor` | ||
|
||
public const string BackgroundPanel = "AngleRect"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
holdover from ass naming conventions in StyleNano
. I'll rename it for consistency after resolve merge conflicts.
public ColorPalette PositiveButtonPalette { get; } | ||
public ColorPalette NegativeButtonPalette { get; } | ||
|
||
public StyleBox ConfigureBaseButton(IStyleResources sheet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this really is not a good use of DIMs. I would rather these be extension methods or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be on ButtonSheetlet
anyway probably
Content.Client/Stylesheets/Redux/SheetletConfigs/IPanelConfig'.cs
Outdated
Show resolved
Hide resolved
Most of the trivial changes have been dealt with, waiting on replies to my comments. Also updated the docs PR. I'll finish this up and merge upstream changes tomorrow. |
This PR is a continuation of a derelict PR, #28356.
About the PR
Summary
StyleNano
is GONEPROOF THE PR ACTUALLY WORKS: link
This won't break the game I pinky promise
Why / Balance
space-station-14/Content.Client/Stylesheets/StyleNano.cs
Lines 42 to 1611 in afc8002
Technical details
This literally just rewrites content-side stylesheets from the ground up.
One thing I feel is important to note if you are going back through my commits, is that I first got the sheetlets working as quick as possible (which was mot quick), and then dis multiple refactor passes to make the code better. The current code is pretty good but probably still a refactor or two away from where I'll be completely happy with it.
Design Decisions
This is really long so uh, click to expand
As anyone who's had the misfortune of editing
StyleNano
is probably painfully aware of, the main problem withStyleNano
is how difficult it is to find anything. Maybe the style classes you want are sitting somewhere in that gargantuan tangle of code, or maybe not. Maybe it;s just easier to hardcode everything in the UI code instead (WHICH IS THE ROUTE LOT OF PEOPLE HAVE (rightfully) TAKEN (stylenano was a complete fucking mess)). With this new iteration, I tried to maximize the readability of the structure of the code. Want to know what style classes are available for you to use? Look inStyleClass
. Want to know the styles applicable to labels?LabelSheetlet
orTextSheetlet
. Buttons?ButtonSheetlet
.But what are
Sheetlet
s? Good question person-who-didnt-read-the-original-PR! Basically how styles used to work is every single style rule would be agglomerated in one unholy massive fucking list. This is still how it works, but now the responsibility of chipping in styles to this massive list is spread out among all the sheetlets. They have one method,StyleRule[] GetRules
, and all these rules from all the sheetlets are collected up to do in like 30-40ish files what used to be one 1600 line file.ISheetletConfig
is intended to cut down on repeated code by providing shared functionality between the stylesheets. Its literally only used for buttons. Its also used (in my crusade against anything hardcoded) to store resource paths, because it's easier to reference if it's all centralized.Deviations from Original PR's Direction
The class holding all the style classes,
StyleClass
was originally namedStyleclasses
. I think this was a better name. Unfortunately,Control
s have a field with the exact same name, meaning to referenceStyleclasses
, you would have to type outStylesheets.Redux.Styleclasses.<whatever>
. This syntax would be shortened after everything is moved out ofRedux
but still, annoying. The shorter syntax is nicer. Also as an extra bonus, it's closer to the syntax of something likeStyleClassLabelHeading
. Just add a dot! That's kinda neat.class stopped sounding like a real word. Class class class class class. Blegh.
Instead of an array of colors, I made a custom
ColorPalette
class. This is because when transferring all the styles intoSheetlet
s, I noticed I basically used the palette in one of three ways, foreground elements (Buttons etc.), background elements, (Panels, etc.), and text. So I represented that in the palette! Just makes code a bit more readable / robust. Also, I just kinda hated that[0]
was the brightest and[4]
the darkest. It should be the other way around! and what if I want to add more colors? There are more than five shades of any given color!This does not compromise customizability because any colors can be curly-brace initialized after the fact if you are really motivated.
Also I made a cool shitty codepen to help me pick colors.
What moony originally did in their PR was they had resources scoped / access-locked (which I have kept), and, as a consequence, sheetlets that used a resource would have to be sheet-specific. This included buttons, panels, windows, etc.; things that basically every stylesheet would want, and which would have to be duplicated for every stylesheet. I think the intention behind this is to prevent resources intended for one stylesheet being used unintentionally in another, but honestly, the organization of the resources folder is a mess (a lot of style-generic resources are sitting in
Textures/Interface/Nano
), and this would lead to a lot of copying resources, which is definitely a maintenance hazard. I could clean it up, but then it could be a NIGHTMARE to merge for downstream forks (probably idk) (also touching the resource folder scares me). The resource-scoping system does allow you to specify multiple scopes to try in order, but that's kinda icky. So! I propose (and have already implemented) the following solution:GetResourceOr
(and
GetTextureOr
)It gets the scoped resource like normal, but if it doesn't exist, falls back to an absolute root. Is it a generic resource that has no business being is
Textures/Interface/Nano
? I don't care! It can stay there! I really can't be bothered to move it! Now the sheetlet will work with any stylesheet. If the scope happens to exist, it'll use that instead.This method is kinda clunky but that's good (maybe?) because you want to use it where it counts. If there's a resource that, ideally, every theme should have a unique implementation of, then you would just use
GetResource
.Significant Interface Changes
StyleNano
correctly, as I was pretty methodical with it, but its also entirely possible I forgot something. If anybody notices UI that looks worse than they remember, double check it actually is different (this happened to me several times, some of the UIs are just kinda bad), and pretty please PR the changes (or ask me, and I'll probably do it).StyleSpace
now usesSheetSystem
which is likeStyleSpace
but the colors are slightly differentBreaking Changes
StyleNano
is gone!!StyleNano
have been moved either toContent.Client.Stylesheets.Redux.StyleClass
or their associatedControl
.StyleRule
additions toStyleNano
must be rewritten to conform to the new format (See guide (TODO: LINK GUIDE))Content.Client.Stylesheets.Redux.StyleClass
or create a new Sheetlet (Again, see guide)StyleSpace
is gone too, but that probably doesn't affect youAlso
square.png
has been deleted fromTextures/Interface/Nano
. This shouldn't have even been used in the first place;StyleBoxFlat
exists.Future work
Content.Client/Stylesheets/Redux
for ease of review / mergingTODO
s left scattered around for hardcoding and stuffDefaultWindow
Changelog
🆑