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

VC crash screen + load progress #2153

Open
wants to merge 5 commits into
base: Dev
Choose a base branch
from

Conversation

Rebbacus
Copy link

A port of the VC functions I did for the Majora's Mask Randomizer VC - load progress of the ROM and custom crash screens.

The PPC assembly was converted to gzi commands and is placed in the unused spaces of app1. The hooks are in the exception handler, the start-up and rom loading routines, and in cpuFindFunction to save a register value when it finds an illegal instruction.
I've played for a little bit on dolphin and wii and it seems okay.

The wii crash addresses, N64 registers and last entered recomp node
NICEcrash01

An N64 stack trace is attempted. Due to how the VC doesn't really keep track of the program counter, a guess is made on where to start. This often fails however, and can result in incorrect info or the page showing unavailable.
NICEcrash02

Wii GP registers
NICEcrash03

Wii Back Chain
NICEcrash04

Some recomp code heap stuff
NICEcrash05

Couldn't get controller polling to work, so the crash pages advance on a timer, and this doesn't include any random crash patches yet - I'd like to see what crashes get reported first.

@fenhl fenhl added Type: Bug Something isn't working Type: Enhancement New feature or request Component: Patching Affects the patching of the ROM labels Dec 21, 2023
@fenhl fenhl added Status: Needs Review Someone should be looking at it Status: Needs Testing Probably should be tested Status: Under Consideration Developers are considering whether to accept or decline the feature described labels Feb 15, 2024
@fenhl
Copy link
Collaborator

fenhl commented Apr 22, 2024

@Rebbacus Could you post a screenshot of the load screen as well please?

@Rebbacus
Copy link
Author

This will come up after the "You will need the classic controller" message when starting up the VC, and after bringing up the home menu.
Screenshot 2024-04-22 163842

@fenhl
Copy link
Collaborator

fenhl commented Apr 30, 2024

We've been discussing this internally and some concerns have been raised:

  1. The comments make it look like this is removing the existing features for using 8MB memory and remapping the D-pad. I'm assuming this isn't actually the case, but could the comments be adjusted to directly point to where these features have been moved to?
  2. The comments say it is for NACE, but if it doesn't play nice with NACJ then it'll need to be split off into its own .gzi with the correct one being determined somehow.

@fenhl fenhl added the Status: Waiting for Author Changes or response requested label Apr 30, 2024
@Rebbacus
Copy link
Author

  1. The D-pad and 8MB memory lines were present, but I've moved them up to the top with the comments for visibilty.

  2. I was under the impression the patcher was only taking US OoT wads, since the requirement pages stated it wanted the US OoT wad. Oops.
    This gzi is only good for the US version, as the offsets for the JP wad differ. I'm not sure how to get the patcher to tell them apart.

@fenhl fenhl removed the Status: Waiting for Author Changes or response requested label May 27, 2024
Main.py Outdated
Comment on lines 277 to 288
if wad_app1_sha1 in wad_app1_sha1_usa:
if wad_app5_sha1 in wad_app5_sha1_usa:
wad_patch_name = "ootr_usa.gzi"
else:
raise Exception('Base WAD file is not a valid OoT USA or JPN wad.')
elif wad_app1_sha1 in wad_app1_sha1_jpn:
if wad_app5_sha1 in wad_app5_sha1_jpn:
wad_patch_name = "ootr_jpn.gzi"
else:
raise Exception('Base WAD file is not a valid OoT USA or JPN wad.')
else:
raise Exception('Base WAD file is not a valid OoT USA or JPN wad.')

Choose a reason for hiding this comment

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

I think that instead of having a bunch of branches here,you should pull out the values being checked into individual booleans e.g.

isUsaApp1 = wad_app1_sha1 in wad_app1_sha1_usa
isUsaApp5 = ...
isUsaWad = isUsaApp1 && isUsaApp5
[...]
if isUsaWad:
    wad_patch_name = ...
elif isJapanWad:
    [...]
else:
    raise Exception(...)

Also the exception should either be a user defined class derived from Exception, RuntimeException, or one of the other concrete exceptions. You should not use the raw Exception base class.

Copy link
Author

Choose a reason for hiding this comment

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

That makes more sense, sure 1841b22

Choose a reason for hiding this comment

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

Ah, you should probably still use the repo's standard for variable naming. I wrote them using camelcase in the suggestion out of habit from work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Patching Affects the patching of the ROM Status: Needs Review Someone should be looking at it Status: Needs Testing Probably should be tested Status: Under Consideration Developers are considering whether to accept or decline the feature described Type: Bug Something isn't working Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants