Skip to content

Fix concurrent do-while instances only running do section once#8263

Merged
sovdeeth merged 5 commits intodev/patchfrom
patch/fix-do-while
Nov 11, 2025
Merged

Fix concurrent do-while instances only running do section once#8263
sovdeeth merged 5 commits intodev/patchfrom
patch/fix-do-while

Conversation

@UnderscoreTud
Copy link
Copy Markdown
Member

Problem

If two or more do-while calls of the same instance happen at the same time, the do section only works properly for the first one.

Solution

Use a set to track which events have run the do section instead of a single boolean variable

Testing Completed

Manual testing

Supporting Information

N/A


Completes: #8262
Related: none

@UnderscoreTud UnderscoreTud requested a review from a team as a code owner October 30, 2025 09:35
@UnderscoreTud UnderscoreTud requested review from Burbulinis and cheeezburga and removed request for a team October 30, 2025 09:35
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Oct 30, 2025
@UnderscoreTud UnderscoreTud added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Oct 30, 2025
@UnderscoreTud UnderscoreTud moved this to In Review in 2.13 Releases Oct 30, 2025
Copy link
Copy Markdown
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiter! Waiter! More tests please!

@UnderscoreTud
Copy link
Copy Markdown
Member Author

Waiter! Waiter! More tests please!

I didn't know how to do tests for this since iirc you can't have delays in tests

@sovdeeth
Copy link
Copy Markdown
Member

Just have a function with a do while, call the function twice
Shouldn't need to wait for a delay?

Copy link
Copy Markdown
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be okay with sovde's suggestion.

@skriptlang-automation skriptlang-automation bot removed the needs reviews A PR that needs additional reviews label Oct 30, 2025
@UnderscoreTud
Copy link
Copy Markdown
Member Author

UnderscoreTud commented Oct 30, 2025

Just have a function with a do while, call the function twice Shouldn't need to wait for a delay?

That won't properly test it, because without a delay the second loop will only run after the first function call has finished execution. As seen here:

function test(id: text):
    do while {_a} < 20:
        broadcast {_id}
        add 1 to {_a}

on load:
    test("1")
    test("2")
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 1
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2
[17:54:50 INFO]: 2

@sovdeeth
Copy link
Copy Markdown
Member

Yeah but can't you have the delay exist but not rely on it finishing?

Eg
func():
Do while x is y:
add 1 to {}
Wait 1 tick

And just ensure the {} = number of times func is called

@skriptlang-automation skriptlang-automation bot added the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Nov 2, 2025
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.13 Releases Nov 2, 2025
@sovdeeth sovdeeth merged commit 177ae17 into dev/patch Nov 11, 2025
5 checks passed
@skriptlang-automation skriptlang-automation bot added the completed The issue has been fully resolved and the change will be in the next Skript update. label Nov 11, 2025
@github-project-automation github-project-automation bot moved this from Awaiting Merge to Done - Awaiting Release in 2.13 Releases Nov 11, 2025
@skriptlang-automation skriptlang-automation bot removed the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Nov 11, 2025
@sovdeeth sovdeeth moved this from Done - Awaiting Release to Done - Released in 2.13 Releases Dec 4, 2025
@UnderscoreTud UnderscoreTud deleted the patch/fix-do-while branch December 9, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update.

Projects

No open projects
Status: Done - Released

Development

Successfully merging this pull request may close these issues.

3 participants