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.

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

Add a system for modifying entity names without causing conflicts #27863

Merged
merged 32 commits into from
Jun 16, 2024

Conversation

Tayrtahn
Copy link
Member

@Tayrtahn Tayrtahn commented May 9, 2024

Requires space-wizards/RobustToolbox#5216

About the PR

Adds NameModifierSystem, which aims to fix the problems that arise from different systems renaming entities.

Modifies a bunch of systems that previously could cause renaming conflicts when used on the same entity. Specifically, this fixes issues with metamorphic glasses, glue, lube, flares/glowsticks, forensic pads, DNA scrambler implants, PAIs, baby animals, borgs, zombies, cluwnes, chameleon clothing, and the admin renaming command and verbs.

Also includes ModifyWearerName Component/System both as a demo and to make it easier to add clothing that does simple name modifications. It's not used on anything in this PR though.

Why / Balance

See #27670 for an explanation of the issues that can come up with multiple systems renaming things. In short, the old method for renaming an entity was to copy the old name into a component field and then copy it back to revert the name. This meant that any additional changes to the name between those actions would be bulldozed. This system prevents that and makes it easy to add and remove name modifications. You can now have a "glued lubed mojito glass (for nerds)" and refill it to make a "glued lubed screwdriver glass (for nerds)" without issues.

Fixes #27670

Technical details

When NameModifierSystem.RefreshNameModifiers is called, a RefreshNameModifiersEvent is passed around the entity and any subscribed components can add modifiers to to it. The event is also relayed to equipped items.

A bunch of existing systems that temporarily changed entity names have been modified to use NameModifierSystem instead. Systems that make permanent changes still work and will have their changes respected by this system.

This PR also includes ModifyWearerNameComponent/System, but doesn't use it on anything. I used it quite a bit in testing, and it nicely demonstrates the integration with InventoryRelayEvent. It's a simple component that can be added to clothing prototypes to make them add a prefix, postfix, or override to the wearer's name while equipped. For example, the lawyer's badge can add the "Attorney" title to a wearer's name with just a little yml:

- type: ModifyWearerName
  locId: lawyer-name-prefix

Media

Multiple modifiers surviving changes:
Screenshot 2024-05-06 at 5 47 22 PM
Screenshot 2024-05-06 at 5 49 15 PM

This may not make a lot of sense, but at least the name behaves properly:
Screenshot 2024-05-06 at 5 50 45 PM

ModifyWearerNameComponent:

Screen.Recording.2024-05-07.at.7.30.02.PM.mov
  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

Systems that apply reversible changes to entity names should be changed to use NameModifierSystem so they don't conflict with other name changes.

Changelog

Nah.

@github-actions github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted label May 9, 2024
@Tayrtahn Tayrtahn marked this pull request as draft May 24, 2024 20:27
@Tayrtahn
Copy link
Member Author

Just realized that specifying prefix/postfix in the code is bad for localization, since what might be a prefix in one language can be a postfix in another ("lubricated crowbar" -> "palanca lubricada").

I'll rework this to ditch the prefix/postfix concept and use Loc strings for formatting.

@Tayrtahn Tayrtahn added the S: Needs Engine PR Merged Status: Requires an existing Robust Toolbox PR to be merged first. label Jun 7, 2024
@Chief-Engineer Chief-Engineer removed their request for review June 11, 2024 15:41
Copy link
Member

@AJCM-git AJCM-git left a comment

Choose a reason for hiding this comment

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

This is looking good, just some details, also I think master RT submodule still doesnt have the required merged pr
Also, remember to write something for codebase changes after this is merged

@AJCM-git AJCM-git added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Jun 13, 2024
@Tayrtahn Tayrtahn requested a review from AJCM-git June 13, 2024 18:20
@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Jun 13, 2024
@Tayrtahn Tayrtahn added S: Awaiting Changes Status: Changes are required before another review can happen S: Needs Engine Update Status: Requires a newer version of the Robust Toolbox engine. S: Needs Review Status: Requires additional reviews before being fully accepted and removed S: Needs Engine PR Merged Status: Requires an existing Robust Toolbox PR to be merged first. S: Needs Review Status: Requires additional reviews before being fully accepted S: Awaiting Changes Status: Changes are required before another review can happen S: Needs Engine Update Status: Requires a newer version of the Robust Toolbox engine. labels Jun 13, 2024
@Tayrtahn Tayrtahn requested a review from AJCM-git June 15, 2024 23:14
Copy link
Member

@AJCM-git AJCM-git left a comment

Choose a reason for hiding this comment

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

looks aight

@AJCM-git AJCM-git added S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. and removed S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. labels Jun 16, 2024
@AJCM-git AJCM-git merged commit 89a9f07 into space-wizards:master Jun 16, 2024
12 checks passed
@Tayrtahn Tayrtahn deleted the renamer-system branch June 16, 2024 20:37
@Tayrtahn Tayrtahn mentioned this pull request Jun 17, 2024
1 task
Erisfiregamer1 pushed a commit to The-Arcadis-Team/arc-station-14 that referenced this pull request Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: Needs Review Status: Requires additional reviews before being fully accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Entity rename mechanics are incompatible with each other
2 participants