-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Further improve NPC trade labels and comments #493
base: master
Are you sure you want to change the base?
Conversation
Slightly off-topic, would you consider using constants in
I used these constant names in Pokegreen, see https://github.com/Narishma-gb/pokegreen/blob/24ad50fa9c7295e7b2fd003a7b40386cc02a7d32/engine/events/in_game_trades.asm |
@SatoMew Can you please provide a bit of info around some of your your actions. For example: Why did you close this? Why did you reopen it? What changed? Narishma-gb made a correction which you appear to agree with (you even already resolved the thread) but you never pushed any additional changes. |
I felt the need to reconsider. That's all I would like to share. |
Any update? |
I like the new explanation. It addresses 2 possible inaccuracies:
The opposite is just as likely. After they fixed the routine, they decided to use Red/Green trade set, and left the code untouched because it did not break anything. Some time later came the final names.
Red/Green also has 3 dialog sets, ranging from casual to more formal. |
This PR partially amends #384 by correcting the information about the previously mentioned localization oversight and reworks the documentation of
InGameTrade_CheckForTradeEvo
a bit.Should
evolve_trade.asm
be renamed to avoid confusion withEVOLVE_TRADE
since the file is only forInGameTrade_CheckForTradeEvo
?