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.

Sign up for GitHub

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

Jump to bottom

[BUGFIX] Fix the song being completable by holding the Window Title Bar #4013

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

[BUGFIX] Fix the song being completable by holding the Window Title Bar #4013

wants to merge 1 commit into from

Conversation

KoloInDaCrib
Copy link
Contributor

@KoloInDaCrib KoloInDaCrib commented Jan 20, 2025 •
edited
Loading

Does this PR close any issues? If so, link them below.

4005

Briefly describe the issue(s) fixed.

While observing the behavior of how lime handles this kinds of things, I stumbled upon the fact that the Thread that updates the Discord RPC wasn't being affected in any way by dragging the Title Bar of the Window. The bugfix uses the same principle - it creates a Thread that repeats infinitely (every 0.1 seconds since that's max elapsed time according to Flixel) and checks if the main thread is frozen. If it is, the game is paused.
Doesn't work on web builds though! I'll see if there is any way to fix the web exclusive autocomplete bug once I figure out how to compile the game in html5 with a scrollbar so I can actually test it lmao

Include any relevant screenshots or videos.

2025-01-20.22-09-27.mp4

@github-actions github-actions bot added size: medium A medium pull request with 100 or fewer changes. pr: haxe PR modifies game code. and removed size: medium A medium pull request with 100 or fewer changes. labels Jan 20, 2025
EliteMasterEric
EliteMasterEric requested changes Jan 21, 2025
View reviewed changes
Copy link
Member

@EliteMasterEric EliteMasterEric left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Aside from some code cleanup, this is pretty slick!

EDIT:

once I figure out how to compile the game in html5 with a scrollbar so I can actually test it lmao

Just edit the HTML/CSS of the page after you compile the app

source/funkin/play/PlayState.hx Show resolved Hide resolved
@EliteMasterEric EliteMasterEric added type: minor bug Involves a minor bug or issue. status: needs revision Cannot be approved because it is awaiting some work by the contributor. labels Jan 21, 2025
@EliteMasterEric EliteMasterEric self-assigned this Jan 21, 2025
@github-actions github-actions bot added size: large A large pull request with more than 100 changes. and removed size: large A large pull request with more than 100 changes. labels Jan 21, 2025
@Hundrec Hundrec added the size: large A large pull request with more than 100 changes. label Jan 22, 2025
@NotHyper-474
Copy link
Contributor

Something funny that happened when I was testing this (enable audio)
The game stuttered right when Dad started singing, so it paused but didn't stop his vocal.

8mb.video-331-RZeI91Ha.mp4

@KoloInDaCrib
Copy link
Contributor Author

KoloInDaCrib commented Jan 22, 2025 •
edited
Loading

Something funny that happened when I was testing this (enable audio) The game stuttered right when Dad started singing, so it paused but didn't stop his vocal.

8mb.video-331-RZeI91Ha.mp4

oh huh, guess I gotta increase the treshold for the side thread updating /init the side thread from somewhere else since by the looks of it only the main thread lags behind for more than 0.1 seconds

Edit: looks like pausing happens for any kind of stutter, whoops! draft it is!

@KoloInDaCrib KoloInDaCrib marked this pull request as draft January 22, 2025 20:00
AbnormalPoof
AbnormalPoof reviewed Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@AbnormalPoof AbnormalPoof left a comment

Choose a reason for hiding this comment

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

ThreadUtil looks really useful for mods! I suggest adding a function like createThread() so scripts can create a thread without having to make it delayed or loooping.

@KoloInDaCrib KoloInDaCrib marked this pull request as ready for review January 23, 2025 06:17
@KoloInDaCrib
squashed commits
ef26a62 
extend treshold + createThread in threadUtils

add threadutil

use threads to fix a small bug
@AbnormalPoof
Copy link
Collaborator

Would it be possible to implement mutex (sys.thread.Mutex) support into ThreadUtil as well?

@AbnormalPoof AbnormalPoof added the topic: platform specific Specific to only one or several of the game's supported targets. label Jan 24, 2025
@EliteMasterEric EliteMasterEric added this to the 0.7.0 (???) milestone Jan 24, 2025
@EliteMasterEric
Copy link
Member

v0.7.0 will have a bunch of threading code in it so this will have to be reconciled with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AbnormalPoof AbnormalPoof AbnormalPoof left review comments

@EliteMasterEric EliteMasterEric EliteMasterEric requested changes

Assignees

@EliteMasterEric EliteMasterEric

Labels
pr: haxe PR modifies game code. size: large A large pull request with more than 100 changes. status: needs revision Cannot be approved because it is awaiting some work by the contributor. topic: platform specific Specific to only one or several of the game's supported targets. type: minor bug Involves a minor bug or issue.
Projects
None yet
Milestone
0.7.0 (???)
Development

Successfully merging this pull request may close these issues.
5 participants
@KoloInDaCrib @NotHyper-474 @AbnormalPoof @EliteMasterEric @Hundrec