-
Notifications
You must be signed in to change notification settings - Fork 42
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
Multiple initiative rolls at the same time result in lost rolls #27
Comments
cant do much about it with how emojis work and the character limits on
messages. results are tagged for each person too....
…On Wed, Jun 17, 2020 at 9:27 PM Timothy Regan ***@***.***> wrote:
How to reproduce:
Make two initiative rolls in quick succession:
[image: proof]
<https://user-images.githubusercontent.com/5122881/84970731-46ce1180-b0e9-11ea-80dd-677791efedbe.PNG>
For logging purposes, "Today" is 6/17/2020
Recommendation is to build a patch for the initiative order, and then
merge it with whatever the current order is at the end.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#27>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWUO6EXNV74QE2HMTWJAZ3RXF3QNANCNFSM4OBERX5A>
.
|
I don't think that would matter, would it? The issue seems to be the time it takes to complete the roll. It gets the init order at the start, performs all the steps needed, and then replaces the init already stored with the one it builds. With two running at the same time there's a high chance that one will overwrite the other. |
oh, no. that won't happen results are written before the messages are sent
…On Wed, Jun 17, 2020 at 9:34 PM Timothy Regan ***@***.***> wrote:
I don't think that would matter, would it? The issue seems to be the time
it takes to complete the roll. It gets the init order at the start,
performs all the steps needed, and then replaces the init already stored
with the one it builds. With two running at the same time there's a high
chance that one will overwrite the other.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWUO6EFCGVNDHYYG4ALB2TRXF4JTANCNFSM4OBERX5A>
.
|
Doesn't the roll function send messages while it does its thing? |
no, well yes, but each has its own process. i dont think you could send commands fast enough to confuse the bot/db
…On Wed, Jun 17, 2020 at 9:37 PM Timothy Regan ***@***.***> wrote:
Doesn't the roll function send messages while it does its thing?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWUO6HFJCMKFLGOCDTNOFDRXF4W5ANCNFSM4OBERX5A>
.
|
It's distressingly easy. Maybe something has changed about the data api? |
oh yeah, this quick again. lemme sleep on it. till then roll slower |
Appreciate you taking a look at it! Looks like the other process is waited on. That would mean that it has to finish doing the roll messages before it updates the init:
|
Ahhhh, if is one of those "Do I need this code? No I do not need this code.
I have deleted the code. Oh shit I need that code!" Moments.
I think I know the fix, will test in the morning.
…On Wed, Jun 17, 2020, 9:43 PM Timothy Regan ***@***.***> wrote:
Looks like the other process is waited on. That would mean that it has to
finish doing the roll messages before it updates the init:
https://github.com/SkyJedi/FFGNDS-Discord-Dice-Roller/blob/926c0a290c2874a484ad45d3a715ef95e84cada6/modules/SW.GENESYS/initiative.js#L25
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWUO6AK6X5NETZ23N3G6TLRXF5M3ANCNFSM4OBERX5A>
.
|
its _better_but not bullet proof. I need to completely overhaul initiative to really fix the problem. so now just have players space out imitative rolls |
Cool, iterative fixes get us there eventually. |
How to reproduce:
Make two initiative rolls in quick succession:
For logging purposes, "Today" is 6/17/2020
Recommendation is to build a patch for the initiative order, and then merge it with whatever the current order is at the end.
The text was updated successfully, but these errors were encountered: