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

Trapped doors reset if you close the door then re-enter the level in multiplayer #5184

Closed
ephphatha opened this issue Aug 3, 2022 · 6 comments · Fixed by #5838
Closed

Trapped doors reset if you close the door then re-enter the level in multiplayer #5184

ephphatha opened this issue Aug 3, 2022 · 6 comments · Fixed by #5838
Assignees
Labels
bug Something isn't working vanilla Bugs found also in the original Diablo 1.09b release

Comments

@ephphatha
Copy link
Contributor

Operating System

Windows x64

DevilutionX version

Other (please specify version number)

Describe

UpdateTrapState returns early if it uses a door as a trigger and the door is closed. This means that in a multiplayer game a player can set off a trap twice by leaving the level and replaying the delta

To Reproduce

Start or join a multiplayer game
find a level with a trapped door
open the door, close it, and observe the door is "Closed door" and not "Trapped door" (if playing as a rogue)
Leave the level then re-enter
Observe the door label is "Trapped door" (if playing as a rogue)
Open the door, note the trap fires again

Expected Behavior

Traps on doors should only trigger once

Additional context

I suspect this can happen in vanilla but haven't found a trapped door yet to be able to test.

@ephphatha
Copy link
Contributor Author

Actually this might be because close door messages aren't processed when replaying the delta? Probably need a handler to clear trap flag so the door is marked as being touched

@qndel
Copy link
Member

qndel commented Aug 3, 2022

For easy testing in vanilla:
0x441CC2 - C7 45 F4 0A000000 - mov [ebp-0C],0000000A { 10 }
0x441CC2 - C7 45 F4 64000000 - mov [ebp-0C],00000064 { 100 }
= guaranteed traps :P

@qndel qndel added bug Something isn't working vanilla Bugs found also in the original Diablo 1.09b release labels Aug 3, 2022
@qndel
Copy link
Member

qndel commented Aug 3, 2022

Confirmed vanilla bug

@AJenbo
Copy link
Member

AJenbo commented Aug 3, 2022

if you can review #5169 then it should be easy for me to fix it, with out conflicts :)

@AJenbo
Copy link
Member

AJenbo commented Aug 3, 2022

I wonder if this is also the case for chest using the Thaumaturgic shrines

@ephphatha
Copy link
Contributor Author

ephphatha commented Aug 3, 2022

I wonder if this is also the case for chest using the Thaumaturgic shrines

Looks like it could be an issue depending on the order messages are played back. In vanilla if a chest spawns after the shrine it will get it's state changed after the shrine refill logic runs so won't be correct. With master using unordered object deltas devx is harder to predict, we would need to order the messages or somehow prevent the change to selflag while ensuring trapflag is updated.

Edit: guess that's why they're singleplayer only 😂, would be nice to solve that and let them spawn in multi.

Been meaning to finish reviewing that PR as well, it's an open tab 😅.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vanilla Bugs found also in the original Diablo 1.09b release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants