Skip to content
This repository has been archived by the owner on May 16, 2024. It is now read-only.

Giant negative movement numbers in 5E 1.2.0, 0.7.8 #16

Closed
apoapostolov opened this issue Dec 17, 2020 · 8 comments · Fixed by #21
Closed

Giant negative movement numbers in 5E 1.2.0, 0.7.8 #16

apoapostolov opened this issue Dec 17, 2020 · 8 comments · Fixed by #21
Labels
bug Something isn't working

Comments

@apoapostolov
Copy link

apoapostolov commented Dec 17, 2020

For most of my actors, there are no issues but this particular actor is having massive constantly increasing negative numbers for all speeds anytime the actor's inventory is changed. The effects keep multiplying in that user and they cannot be deleted.

Export of the 5E actor: https://we.tl/t-Oqq6jmEfe6

@apoapostolov apoapostolov changed the title Giant negative movement numbers Giant negative movement numbers in 5E 1.2.0, 0.7.8 Dec 17, 2020
@paullessing
Copy link
Contributor

I have the same issue in 0.7.9; the "lightly encumbered" and "heavily encumbered" effects keep getting added and never removed, so I end up with tens of stacked "encumbered" effects.

@VanirDev
Copy link
Owner

VanirDev commented Jan 3, 2021

Known issue with the current implementation. I'm working on a fix but the holidays have kept me busy unfortunately.

@VanirDev VanirDev added bug Something isn't working In Progress This issue is currently being worked on labels Jan 3, 2021
@paullessing
Copy link
Contributor

I've confirmed that this is a race condition caused when there are multiple clients open at the same time (which happens when there are multiple users logged in).

When an item is updated or added/removed, all clients simultaneously attempt to set the encumbrance, which causes them to stack. Because the current logic removes all previous effects and then adds its own, the stacking always happens whenever there are multiple clients open.

I attempted to fix this by updating the first found existing effect instead, but that does not solve the race condition that occurs when multiple clients try to create the effect at the same time, for the first time, or when switching between unencumbered and encumbered.

@VanirDev do you want to have a chat about how to solve this? I'm happy to have a stab at it on my own, but I'm new to Foundry development and you may have some insight that I don't. Contact me at paul@<myusername>.com if you're interested, otherwise I'll open another pull request once I've got a solution.

@paullessing
Copy link
Contributor

I've got a solution, and will be raising a pull request early this week once I've cleaned up my code.

@VanirDev
Copy link
Owner

I've got a solution, and will be raising a pull request early this week once I've cleaned up my code.

Great work! I'd love to see how you resolved the race condition. I've been frustratingly trying to find a way to execute code from the server's perspective instead of on a per-client basis, alas that doesn't seem possible.

@VanirDev
Copy link
Owner

VanirDev commented Jan 11, 2021

I've confirmed that this is a race condition caused when there are multiple clients open at the same time (which happens when there are multiple users logged in).

When an item is updated or added/removed, all clients simultaneously attempt to set the encumbrance, which causes them to stack. Because the current logic removes all previous effects and then adds its own, the stacking always happens whenever there are multiple clients open.

This is the same behaviour that is causing the error notifications that people are seeing such as this #20, as when a client doesn't have permission to update the actor, it'll attempt to do so anyway and produce the error.

@paullessing
Copy link
Contributor

PR is open; I've got a game tomorrow night where I will be testing this.

@VanirDev
Copy link
Owner

PR is open; I've got a game tomorrow night where I will be testing this.

Thanks for this, I'll review it tomorrow when I have a chance. Let me know how your session goes, I'll probably use my own players as guinea pigs come Wednesday.

@VanirDev VanirDev removed the In Progress This issue is currently being worked on label Jan 17, 2021
@VanirDev VanirDev unpinned this issue Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
3 participants