search

League-of-Foundry-Developers / fvtt-module-popout

FVTT Module PopOut!
Other
29 stars 24 forks source link

Add setting to customize the delay before loading the sheet. #13

Closed Dudrie closed 4 years ago

Dudrie commented 4 years ago

Because my players could not load the popout with the PF2e system I added a timeout setting. This setting can be used to increase the delay between opening the popout and loading the actual sheet.

While https://github.com/kakaroto/fvtt-module-popout/issues/12#issuecomment-629046073 mentions that this might be an issue with the system (even though it is related to PF1) I think that this setting is a great escape hatch for such cases. This makes this module a bit more independent from other modules by providing a workaround if needed.

If this setting is unwanted feel free to reject this PR.

kakaroto commented 4 years ago

i'm not really a fan of this change because it's a workaround for an issue rather than a fix for an issue (which is external to the project). So far system developers have fixed their stuff quickly enough and it usually involved just moving one line further down in their code. Also, exposing such a setting to users can cause more problems than resolve them, because some users (to be fair, the majority) may not understand what that setting does and will change it to ridiculous values which will either break things (would a NaN still fire the timeout?) or make the sheet open a minute later and they'd be yelling as to why popout isn't working, without realizing they changed a setting they had no idea what it was for and caused the misbehavior themselves. I'm leaving the PR open for now so I can look at this further when I get more time, but I just wanted to share my thoughts as it's been over a week and I didn't yet get the time to look it over fully.

Dudrie commented 4 years ago

I agree to your thoughts and concerns. I have not thought about the issue that this workaround might introduce confusion for users.

A few ideas if one would implement this: Edge cases like NaN or arbitrary values could be handled by falling back to the default value. Is Foundry able to show some "Loading assets" while the popout is "loading" (ie "waiting").

kakaroto commented 4 years ago

For future reference (as I've looked it up a few times), discord thread discussing this issue : https://discordapp.com/channels/170995199584108546/697845559435853865/718568831664259223