fix(rooms): cast half enters unlocked, card art, no duplicate triggers - #15704
fix(rooms): cast half enters unlocked, card art, no duplicate triggers#15704maff1989 wants to merge 3 commits into
Conversation
- Casting a Room half now auto-unlocks that door on ETB as the rules require. Previously the permanent entered fully locked, neither half active, forcing manual re-unlock. - Card art now renders on the battlefield. The permanent's name is modified by RoomCharacteristicsEffect (showing only unlocked halves), but the client looked up card images by that modified name. Now uses the full split-card name from the blueprint. Additionally falls back to collector-ID-inclusive filenames for image sets that include them. - Fixed compounding trigger registrations. Room halves would fire 2× after first unlock, 3× after both unlocked. Three interacting bugs: restoreUnlockedStats() skipped game-state trigger registration for non-copy permanents; half-card triggered abilities leaked into the trigger map during game init; addRoomCharacteristics() re-registered all half abilities on every game-state reset cycle, duplicating the entries restoreUnlockedStats() had already re-added.
|
I've done some further testing today and it appears this is NOT working anymore. I'm not sure what happened. I'll close this one and open a new one once I've done some further testing and verify it works consistently. |
|
Create and use unit tests to check (it must fail on master and pass on PR). It allow to collect client side info with image data -- so whole image part is testable. See example with |
|
For duplicated triggers - if it really duplicated somehow then unit tests will ask about triggers order and fail on miss choice on |
|
Yeah the current implementation of rooms is weird, in my experience it's inconsistent if the room enters unlocked or locked. It seems like it's something that is going to be challenging to resolve because of how strange a bug it is, because it varies card to card at least. |
… permanent entries RoomEnterUnlockEffect.apply() cleared lastCastHalf on the shared RoomCard blueprint after the first ETB. This caused complex rooms (more triggers, more applyEffects() cycles) to lose the cast-half info during state reconstruction, while simple rooms with few effects could sometimes squeak through — producing the card-to-card inconsistency the devs observed. Fix: delete the clear. ZonesHandler already clears lastCastHalf on zone transitions (hand, graveyard, exile, library), covering all non-cast entry paths. wasRoomUnlockedOnCast() on the permanent guards duplicate unlocks. - Removed roomCardBlueprint.setLastCastHalf(null) - Collapsed return into bare unlockDoor() call
|
Thanks for the info guys :) I re-opened this PR with an additional commit on the branch that has been working reliably in my local testing. |
There's lots of room-related regressions in the CI run, so your fix is not correct |
…unlock The RoomCard blueprint is a server-singleton shared across all permanents of that card. After the ETB reads lastCastHalf to determine which door to unlock, it must clear the value — otherwise stale cast-half data leaks into copy permanents (Clever Impersonator, Sakashima) that share the same blueprint. ZonesHandler already handles clearing on non-battlefield/stack zone transitions; this clear covers the one remaining window during the ETB itself. Game-state copies of already-unlocked permanents are guarded by wasRoomUnlockedOnCast(). Also drops the unnecessary `return` from the unlockDoor() call. EntersBattlefieldEffect.replaceEvent() ignores the return value of effect.apply(), so returning unlockDoor()'s result was dead code.
|
Thanks @matoro, all 20 tests are passing now. |
|
can you please add a test that demonstrates the issue? it should fail on master and pass with your fix. |
4 participants
Casting a Room half now auto-unlocks that door on ETB as the rules require. Previously the permanent entered fully locked, neither half active, forcing manual re-unlock.
Card art now renders on the battlefield. The permanent's name is modified by RoomCharacteristicsEffect (showing only unlocked halves), but the client looked up card images by that modified name. Now uses the full split-card name from the blueprint. Additionally falls back to collector-ID-inclusive filenames for image sets that include them.
Fixed compounding trigger registrations. Room halves would fire 2× after first unlock, 3× after both unlocked. Three interacting bugs: restoreUnlockedStats() skipped game-state trigger registration for non-copy permanents; half-card triggered abilities leaked into the trigger map during game init; addRoomCharacteristics() re-registered all half abilities on every game-state reset cycle, duplicating the entries restoreUnlockedStats() had already re-added.
Tested in-game with Unholy Annex // Ritual Chamber and confirmed it is working. No other obvious splash damage introduced with these changes.