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

Maybe add comments explaining that stat_multipliers.asm and stat_multipliers_2.asm are stored in different banks #1119

Open
Grate-Oracle-Lewot opened this issue May 9, 2024 · 2 comments

Comments

@Grate-Oracle-Lewot
Copy link

These two tables are identical, which is noted in their comments, but it's not explained why two identical tables are needed (it's because they're in different banks). Speaking from experience, this might lead someone to assume that one table could be deleted and the other reused, resulting in stats being modified based on garbage data. This is particularly insidious because it doesn't cause any build errors and isn't immediately obvious when playing, but in some cases it leads to infinite loops. Suffice to say that a short comment explaining the bank situation would have saved me a whole lot of trouble, and I'd like to save others that trouble.

@Rangi42
Copy link
Member

Rangi42 commented May 16, 2024

Alternatively, what if we reused the same INCLUDE for both tables, moving the StatLevelMultipliers and StatLevelMultipliers_Applied labels outside the INCLUDED file? ( @mid-kid @vulcandth @dannye thoughts?)

@dannye
Copy link
Member

dannye commented May 16, 2024

Big fan of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants