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.

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

[3.x] Backport new pause modes #58519

Closed
wants to merge 1 commit into from
Closed

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Feb 25, 2022

Non-compatibility-breaking backport of #46191

The PR just adds 2 new pause modes:
PAUSE_MODE_STOP_ALWAYS -> same as PROCESS_MODE_DISABLED in 4.0
PAUSE_MODE_PROCESS_PAUSED -> same as PROCESS_MODE_WHEN_PAUSED in 4.0
Everything else is the same.

I tested with AnimationPlayer and the new pause modes work as expected, even when inherited.

@KoBeWi KoBeWi added this to the 3.5 milestone Feb 25, 2022
@KoBeWi KoBeWi requested review from a team as code owners February 25, 2022 01:28
@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 25, 2022

Ok, as it turns out, the fact that it worked for AnimationPlayer was a coincidence. Some nodes seem to just use can_process(), so for them this will work. But some nodes rely on NOTIFICATION_PAUSED and NOTIFICATION_UNPAUSED. The original PR sends the notification after pause mode changes. I could backport that too, but it slightly breaks compatibility I guess 🤔 Not sure if that's acceptable.

EDIT:
I tried to implement it, but it requires more changes and causes more problems than it's worth. Also the current pause modes are broken anyways (#58543, probably applies to all similar nodes)

@akien-mga
Copy link
Member

So it sounds like this isn't working properly yet? Is it still worth attempting for 3.6 or should we shelve it?

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 26, 2022

I think it should work correctly after #58577 is merged.

@akien-mga akien-mga changed the title Backport new pause modes [3.x] Backport new pause modes Apr 25, 2024
@ande8133
Copy link

ande8133 commented Aug 4, 2024

This would be a great feature to have. Any update on this?

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 4, 2024

Well, #58577 was merged, so this might be functional now, but I didn't test. I'm not really interested in this feature anymore, so feel free to take over.

@KoBeWi KoBeWi closed this Oct 26, 2024
@KoBeWi KoBeWi removed this from the 3.x milestone Oct 26, 2024
@KoBeWi KoBeWi deleted the the_world branch October 26, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants