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

3.0 - Microcode overhaul #816

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

3.0 - Microcode overhaul #816

wants to merge 78 commits into from
+11,271 −24,509

Conversation

mountainflaw
Copy link
Contributor

@mountainflaw mountainflaw commented Jul 3, 2024 •
edited
Loading

The goal of this is to combine #789 and #764, while also moving microcode config from the makefile to config_graphics.h

@mountainflaw mountainflaw requested a review from gheskett as a code owner July 3, 2024 16:20
This was referenced Jul 3, 2024
Closed
Closed
@gheskett gheskett added enhancement New feature or request monkaS monkaS cleanup Removal of useless or bloat code/features labels Jul 4, 2024
@gheskett gheskett added this to the 3.0 milestone Jul 4, 2024
@mountainflaw
Copy link
Contributor Author

Fully tested. Ready for review.

arthurtilly
arthurtilly requested changes Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@arthurtilly arthurtilly left a comment

Choose a reason for hiding this comment

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

nice try idiot

actors/metal_box/model.inc.c Outdated Show resolved Hide resolved
arthurtilly
arthurtilly requested changes Jan 23, 2025
View reviewed changes
include/config.h Outdated Show resolved Hide resolved
include/config/config_game.h Outdated Show resolved Hide resolved
include/config/config_graphics.h Outdated Show resolved Hide resolved
include/config/config_safeguards.h Outdated Show resolved Hide resolved
levels/intro/script.c Show resolved Hide resolved
src/game/game_init.c
@@ -33,7 +35,7 @@
#include "emutest.h"

// Emulators that the Instant Input patch should not be applied to
#define INSTANT_INPUT_BLACKLIST (EMU_CONSOLE | EMU_WIIVC | EMU_ARES | EMU_SIMPLE64 | EMU_CEN64)
#define INSTANT_INPUT_BLACKLIST (EMU_CONSOLE | EMU_WIIVC | EMU_ARES | EMU_SIMPLE64 | EMU_CEN64 | DISABLE_INSTANT_INPUT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

whats this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a new config option that forces instant input to be off when defined. made testing a lot easier.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's not referenced anywhere except safeguards as far as i can tell, so it should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/HackerN64/HackerSM64/pull/816/files#diff-544ef2df343d5825e817af38eba0b3908b7d4f1e5d652dce777398032a4c023bR160

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh im blind. still not sure abt it. would rather it was in this file than config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ideally it would get added to something like disable_all in the future, instant input does pop up and cause problems frequently

src/game/rendering_graph_node.c Outdated Show resolved Hide resolved
src/game/screen_transition.c Outdated Show resolved Hide resolved
src/game/screen_transition.c Outdated Show resolved Hide resolved
src/game/screen_transition.c Outdated Show resolved Hide resolved
@mountainflaw
Copy link
Contributor Author

Total changelog with current state of this branch:

  • Removed all microcodes except F3DZEX2, F3DEX3, audio, and of course, boot.
    • All supporting code for removed microcodes has been removed as well.
  • Fixed Goddard framebuffer garbage being visible.
  • Reduced gSPLookAt to one command per frame.
  • Added specular lighting to Goddard screen, among other minor changes for F3DEX3.
  • Added F3DEX3 debug profiler.
  • Added instant input override.
  • Added F3DEX3 lighting engine (enabled by default with F3DEX3), removed mention of wiseguy's in the README.md as this runs faster and outdoes it in every category while remaining vanilla compatible.
  • Uses gSPMemSet on Z buffer for faster clear.

arthurtilly
arthurtilly reviewed Jan 30, 2025
View reviewed changes
include/config/config_graphics.h
* Disables instant input.
* Mostly used for debugging purposes, or for future-proofing a release if concerned.
*/
// #define DISABLE_INSTANT_INPUT
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this into game_init.c and reverse it so it is #define INSTANT_INPUT

arthurtilly
arthurtilly reviewed Jan 30, 2025
View reviewed changes
src/game/mario_actions_cutscene.c
{ (SCREEN_WIDTH * 2), (SCREEN_HEIGHT * 2), G_NEW_MAXZ / 2, 0 }
}
};
#else
Copy link
Collaborator

Choose a reason for hiding this comment

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

was end cutscene viewport ever fixed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

arthurtilly
arthurtilly reviewed Jan 30, 2025
View reviewed changes
src/goddard/renderer.c
vp->vp.vtrans[2] = G_NEW_MAXZ / 2;
#else
vp->vp.vtrans[2] = 0x1FF;
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

these ifdefs here are a bit ugly. if original maxz was 0x3FE then this could be handled above the function and be cleaner? but idk how it works

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah they can be combined i guess

arthurtilly
arthurtilly requested changes Jan 30, 2025
View reviewed changes
Copy link
Collaborator

@arthurtilly arthurtilly left a comment

Choose a reason for hiding this comment

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

shouldve done this as one review but oh well

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

@arthurtilly arthurtilly arthurtilly requested changes

@gheskett gheskett Awaiting requested review from gheskett gheskett is a code owner

@Reonu Reonu Awaiting requested review from Reonu Reonu is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
cleanup Removal of useless or bloat code/features enhancement New feature or request monkaS monkaS
Projects
3.0 (Major Feature Update)
Status: In progress
Milestone
3.0
Development

Successfully merging this pull request may close these issues.
5 participants
@mountainflaw @SonicTheHedgeHog64 @someone2639 @arthurtilly @gheskett