Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
## About The Pull Request This is a revamp of all the parts of the trait system that are connected to the UI. It also replaces the sanitization code that creates the trait list on character load. ## Images   ## Why It's Good For The Game In short: the old trait UI was really confusing and made it hard to discover traits, because you couldn't see what they did without adding them to your character. Bhijn, Marionette and Silicons encouraged me to refactor this. Also, I asked ChatGPT if this change was a good idea and it said yes. ### How It Was #### Visual The window was tiny. The search bar was modal for no real reason. #### Trait Text The traits were in no particular order. You had to click on a trait to learn anything about it other than its name, including: - Its description. - Its cost. - Whether it was excluded by some other trait. Related traits weren't grouped together. Many traits had a positive version that wasn't presented at all, because positive and negative traits were usually not on the same menu. The trait count requirements (things like "neutral traits don't count towards the limit") weren't explained at all. #### Flow Adding a trait closed the window. You then had to reopen the window again to do something else. This would lose your place in the list. Updates to things like currency amount ("how many traits do you have left?") were server-side, so you had to wait for them. #### Maintainability The code to sanitize a character preference was totally separate from the code to apply a list of traits to a character preference, even though these are logically similar operations. The everyone_traits list was confusingly described as a list of neutral traits in some parts of the code and a list of neutral and bad traits in other parts. #### Bugs Traits were double-counted towards the seven-trait limit for characters of custom species, but only the single-count appeared in the UI. The conditions for the currency amounts to be displayed in red (signifying badness) were incorrect, so you'd get incorrect feedback on whether you had screwed up. Photosensitive had a minor bug -- it failed to modify one of the variables it was supposed to. ### Now #### Visual The window is now bigger, with a permanent search bar. It's still smaller than the base character window, so it should look OK on all screens. #### Trait Text Traits are mostly alphabetized (at the top level) and sorted by cost (within each group) with a few exceptions that have obvious contextual reasons related to copy. Each trait is presented with its description and cost. Related traits are grouped together and therefore repetitive text has been moved to the group-level description. Most traits have one-line descriptions. Exclusion info is presented on a tooltip. Because traits take up very little visual space in this design, forbidden traits are displayed on the trait list, with an appropriate warning, unless the forbidden trait is nerd-level Shadekin customization. The trait count requirements (things like "neutral traits don't count towards the limit") are now explained with instant feedback. #### Flow The window now stays open until you're done adding traits. Updates to currency amount happen immediately. You cannot submit an invalid trait allocation, but if you do, it will be rejected by the validator. #### Maintainability All trait adds are handled in one place. All validation is also handled there. The confusingly-defined everyone_traits table is removed. ## Things I Expect To Be Controversial ### Text I changed some copy, mostly to make it shorter. Sorry if I deleted text that you wrote, I wasn't trying to. ### Correctness I touched savegame code. I'm pretty sure my code is correct, but I'd appreciate it if someone else tried to break it. ### Performance Opening the traits screen involves sending probably about 50KB of JSON to the client. That's kind of heavy? I could probably cut it down, but the code would be uglier, and I doubt this is a bottleneck. Character loads are also more expensive now, as they start by computing the trait exclusion table. We could fix this by computing the trait exclusion table on only the traits that the loaded character has -- that would make sanitize_character() comparable in perf to the original version -- but that would take more code, the code would be kind of nonobvious, and it would break my personal vision of a trait deprecation system. ### TGUI My TGUI code is probably not very performant. I wrote it under the assumption that DOM updates are the most expensive and therefore didn't try to optimize anything else. It's also really ugly, but I mostly blame React. I am fairly sure splitting it into more components would not help -- the central problem being solved is algorithmic involving one single nasty data structure, so you'd just get identical code with more data flow issues. ## Problems I Did Not Fix ### Maintainability There are still a few places in the code where a trait is removed without calling into my code. Removing a trait is always allowed, so I'm less worried about this, but in a perfect world we'd change those too. ### Bugs There are some clusters of trait that should be in a mutual exclusion group that I didn't fix: - Colorblindness: These should be exclusive. - Bones: These should be exclusive. There were and still are two versions of Antiseptic Saliva. Someone needs to build a trait deprecation system. Changing from custom species to non-custom species doesn't clear traits. This seems like it _should_ happen -- changing species calls sanitize_everything. I suspect there are deep reasons I don't understand which cause this code path not to be hit, and I would prefer not to change it quite this yet. ## Changelog <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> :cl: Pyrex add: Replaced the trait UI with a prettier one fix: Fixed bugs in Photosensitive refactor: Centralized trait validation from sanitize_character() spellcheck: Weaness should be spelled Weakness /:cl: <!-- Both :cl:'s are required for the changelog to work! You can put your name to the right of the first :cl: if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. -->
- Loading branch information
