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

Common: Use Game loop to render the FFNx logo #543

Merged

Conversation

myst6re
Copy link
Contributor

@myst6re myst6re commented Feb 19, 2023

Summary

  • Fix FFNx logo game freeze by moving it before the squaresoft logo using the same timing logic (the consequence is the publisher movie will be before the FFNx logo)
  • Add the ability to skip the FFNx logo, like the squaresoft logo
  • Fix jp version + hook cleaning

ACKs

  • I have updated the Changelog.md file
  • I did test my code on FF7
  • I did test my code on FF8

@julianxhokaxhiu
Copy link
Owner

julianxhokaxhiu commented Feb 19, 2023

Weird how this is happening to you again. I clearly remember having fixed this on this commit: 08d8092

Are you using the latest master branch code on your own end?

I like the idea of skipping the logo with CTRL+S though, can we implement it generically also for FF7?

@myst6re myst6re force-pushed the fix/ff8-ffnx-logo-freeze branch 2 times, most recently from bae5b74 to e1a83db Compare February 19, 2023 21:19
@myst6re
Copy link
Contributor Author

myst6re commented Feb 19, 2023

Weird how this is happening to you again. I clearly remember having fixed this on this commit: 08d8092

Are you using the latest master branch code on your own end?

Yes, when I start the game without the publisher video (you can rename/remove the file publish.pak to skip this video), and then I try to close the game, it hang. Looking at the code, I can imagine that the game is waiting for the render init to finish before sending any events to Windows.

I like the idea of skipping the logo with CTRL+S though, can we implement it generically also for FF7?

Honestly I don't know, I've done it for FF8 because I know well how pubintro loop works
Note that the logo will be skipped on pressing any button, like with the squaresoft logo

@julianxhokaxhiu
Copy link
Owner

A-ha ok, didn't consider that edge case. But is it a valid one? I'd assume the game to always be installed correctly, and as long as it is it will work fine. I'd prefer finding one way to handle this code instead of having to inject it in custom ways per game edition. Can't this code be independent of the game? that's the main purpose basically of it. Run an intro logo and then start the game.

Regarding the CTRL+S combo we could add it on the renderer loop directly I guess, maybe I can do some tests somewhere this week.

@myst6re myst6re mentioned this pull request Feb 20, 2023
3 tasks
@myst6re
Copy link
Contributor Author

myst6re commented Feb 20, 2023

A-ha ok, didn't consider that edge case. But is it a valid one? I'd assume the game to always be installed correctly, and as long as it is it will work fine. I'd prefer finding one way to handle this code instead of having to inject it in custom ways per game edition.

I checked with the publisher movie, and even the freeze is less noticeable, it is still there.

Can't this code be independent of the game? that's the main purpose basically of it. Run an intro logo and then start the game.

Maybe yes, the missing part of the existing code (the one from the master branch) is that Windows events are not answered during the ffnx logo loop, maybe if we find a function that prevent the window to be unresponsive on each frame, we can keep the existing code :)
Edit: maybe DispatchMessageA
Edit2: yes! But I still think it is best to use the main loop of an existing module instead of recreate our own loop

@julianxhokaxhiu
Copy link
Owner

But I still think it is best to use the main loop of an existing module instead of recreate our own loop

Right, that was something I wanted to tackle because that loop we do block the events parsing you're right. One way would be to hijack one existing loop and "abuse" it, another one is to create our own...at this stage then I'd say to use the game's one, you're right.

May I ask if you could implement the very same logic on FF7 too so we make this PR unique? Would it be possible for you?

Thank you in advance!

@julianxhokaxhiu julianxhokaxhiu added the enhancement New feature or request label Feb 20, 2023
@julianxhokaxhiu julianxhokaxhiu added this to the 1.16.0 milestone Feb 20, 2023
@myst6re
Copy link
Contributor Author

myst6re commented Feb 20, 2023

May I ask if you could implement the very same logic on FF7 too so we make this PR unique? Would it be possible for you?

Thank you in advance!

I will try today, and I'll let you know

@myst6re
Copy link
Contributor Author

myst6re commented Feb 20, 2023

I pushed the FF7 version, they look alike because in both cases I hook to the begin scene call to draw the logo.

@julianxhokaxhiu
Copy link
Owner

Fantastic, thank you! Is it ready for merge? Or do you still have pending fixes?

@myst6re
Copy link
Contributor Author

myst6re commented Feb 21, 2023

Fantastic, thank you! Is it ready for merge? Or do you still have pending fixes?

It is ready :)

@julianxhokaxhiu julianxhokaxhiu changed the title FF8: Fix FFNx logo game freeze Common: Use Game loop to render the FFNx logo Feb 26, 2023
@julianxhokaxhiu julianxhokaxhiu merged commit 3437968 into julianxhokaxhiu:master Feb 26, 2023
@julianxhokaxhiu
Copy link
Owner

Thank you!

ChthonVII added a commit to ChthonVII/FFNx that referenced this pull request May 29, 2023
ChthonVII added a commit to ChthonVII/FFNx that referenced this pull request May 31, 2023
@myst6re myst6re deleted the fix/ff8-ffnx-logo-freeze branch November 3, 2023 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants