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

Various Title Screen Demo Enhancements #833

Open
wants to merge 43 commits into
base: develop/3.0.0
Choose a base branch
from

Conversation

someone2639
Copy link
Collaborator

@someone2639 someone2639 commented Sep 4, 2024

Finally, a PR that nobody asked for!

Summary of changes:

  • The Demo Input segment is no longer allocated if DISABLE_DEMO is defined (0x800 byte memory savings)
  • Demos are disabled by default.
  • Demo files are now in a new text-based camera-agnostic format. Vanilla demos have been converted to this format and are no longer broken.
  • Demo files have been removed from assets.json
  • Added a new file, demo_system.c and .h, and moved demo related functions and variables to that file (Currently 233 lines of stuff that doesn't need to be in a hack without demos).
  • Added a new define, DEMO_RECORDING_MODE, which boots straight to START_LEVEL and prints the new demo format to the debug console.
  • tools/demo_data_converter.py has been rewritten to detect demos in the assets folder, and the hardcoded assets/demo_data.json has been deleted.
  • Added a failsafe to the function that sets the current demo, if DISABLE_DEMO is commented out but no demos are present.

@someone2639
Copy link
Collaborator Author

Ready to review! Still have to test on console to see if my DMA's are valid but I did manage to record some new demos

@someone2639 someone2639 marked this pull request as ready for review September 4, 2024 22:47
@gheskett gheskett added this to the 3.0 milestone Sep 5, 2024
@gheskett gheskett added enhancement New feature or request needs console verification Needs to be tested on console to ensure functionality cleanup Removal of useless or bloat code/features labels Sep 5, 2024
@gheskett gheskett linked an issue Sep 5, 2024 that may be closed by this pull request
@someone2639
Copy link
Collaborator Author

HMC does not crash on console (even after letting it play twice) and all the new demos work too 👍

@someone2639
Copy link
Collaborator Author

This PR is ready for (re-)review. The Yaw/Mag based demo format has been dropped, but every button is now supported, so that developers can still record on their preferred camera system.

@arthurtilly
Copy link
Collaborator

what is the current status of vanilla demos in this PR

@someone2639
Copy link
Collaborator Author

deleted for good since db9a354, because this PR essentially makes them bloat.

@arthurtilly
Copy link
Collaborator

So no demos by default, I assume title screen will handle that correctly and just never attempt to start a demo? But will automatically start if one is made?

@someone2639
Copy link
Collaborator Author

Yes, the new system can handle zero demos (even if the preferred alternative for 0 demos is having them disabled, which removes all the code and the filetable from the final game entirely)

@@ -45,7 +45,7 @@
/**
* Enables a custom, enhanced performance profiler. (Enables PUPPYPRINT by default in config_safeguards).
*/
// #define PUPPYPRINT_DEBUG
#define PUPPYPRINT_DEBUG
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice define change idiot

@@ -18,3 +18,15 @@
* Disables the demo that plays when idle on the start screen (has no effect if KEEP_MARIO_HEAD is disabled).
*/
#define DISABLE_DEMO
Copy link
Collaborator

Choose a reason for hiding this comment

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

If there are no demos by default, this define should be removed tbh

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I reuse this to stub out demo code since I assume the majority of custom games will continue to not use this feature

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would still prefer if it was removed or at least moved out of config into beginning of demo code file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a bit too integral in multiple files (including the link script) to move out of a config header; What if I flip it to like an ENABLE_DEMO_SYSTEM, and have it commented by default instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See how I have it now (on the correct account this time); now it's opt-in

@HackerN64 HackerN64 deleted a comment from farisawan-2000 Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Removal of useless or bloat code/features enhancement New feature or request
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

Title screen demo crash
3 participants