Bionic Spinarette Fix and Trait Merge #2027
Conversation
|
2nd PR, let's go! |
The PoolTestLogHandler was failing tests on all ERROR-level logs, even those without associated exceptions. This was causing numerous false test failures in integration tests. Root cause: The Log() method was calling Assert.Fail() for every ERROR-level log regardless of whether an exception was present. Solution: Added a null check on message.Exception so the test only fails when there's an actual exception to report, allowing informational ERROR logs to be recorded without failing the test. This fixes excessive integration test failures (73 tests) from errors like: - Entity deserialization failures (invalid EntityUid references) - Appearance system resolution issues - Paint visualizer errors during state application
|
Out of Co-Pilot Creds, it's 3AM, last try. Nothing EVER HAPPENS AND ALL THE CHECKS ARE FINE OH MY GOD. (i'm sorry fenn in advance for whatever i managed to do with the co-pilot, just tell me if i need to close this because of the sea of commits i made) |
re-silvered
left a comment
re-silvered
left a comment
There was a problem hiding this comment.
You don't need all the checks to pass for this to be merged, there are just ongoing issues we have to solve with those workflow ran tests that should be done in a separate PR. Otherwise just some attribution to keep things clean and removing the accidentally created file in the working directory. Otherwise good fix!
This reverts commit c706554.
Thanks you! Co-authored-by: re-silvered <254359714+re-silvered@users.noreply.github.com>
…into spinarettefix
Will try to seperately PR the rest of files Co-authored-by: re-silvered <254359714+re-silvered@users.noreply.github.com>
Fardoche0
left a comment
Fardoche0
left a comment
There was a problem hiding this comment.
Cleaned the code by removing and PRing separately PoolTestLogHandler and updated amber.yml thanks to @re-silvered guidance, i would never had managed to clean the code. The time you took to fix and point the small mistakes i made was really helpful and i managed to do so much by myself! I am very grateful, thanks you again.
2 participants
About the PR
Fixed Bionic Spinarette crafts. (along some technical background stuffs.)Why / Balance
Bionic Spinarette was bugged for the longest time! You couldn't weave your silk and it rendered the skill useless, lots of possible interactions and gimmicks are now available!
Technical details
Updated TraitSystem.cs
AddTrait(...) now merges tags from a trait's TagComponent into an existing entity TagComponent instead of replacing it.
This preserves pre-existing tags like SpiderCraft or other permissions while still applying new trait tags.
Added regression coverage in TraitTagMergeTest.cs
Verifies an entity with an existing TagComponent keeps its original tags.
Verifies adding a trait with additional tags adds the new tag without removing the old one.
Verifies whitelist logic recognizes the newly merged tag.
Fixed missing namespace in the new test:
Added using Robust.Shared.Prototypes;
How to test
Start a localhost, grab the trait on a blanket character, go-ingame, spin and weave to your heart content.Media
Changelog
🆑