-
Notifications
You must be signed in to change notification settings - Fork 243
RE: The setting to change the Language of the game to Any language #2441
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
base: Dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preliminary review. I've already mentioned some of this on Discord but am repeating it here for reference.
- The hint area type being language-dependent is going to be an issue for parts of the codebase that don't directly interact with text and therefore don't care about the language. Currently, if I understand the code correctly, these just use an English-language version of the hint area type. This will cause issues if such a hint area value does end up in language-specific code and causes text to be generated in English instead of the correct language. I'd separate the language information from the hint area by passing it as a parameter to methods that generate text.
- Some of the variable names and magic strings (e.g. magic strings called
o,wiw, andcbf; single-letter variable names inLanguage.format_from_textandPatches.create_fake_name) are difficult to understand at a first glance. I would recommend replacing them with longer, more descriptive names. - There seem to be some pieces of logic that check if the language is English and use the logic for Japanese in the
elsecase. The level of Japanese proficiency among maintainers and contributors is much lower than that of English, so for maintainability, English should be the fallback case. - The PR adds
.DS_Storefiles to the repo. These should be removed, and I recommend adding them to your.git/info/excludeto prevent accidentally adding them back in the future. - The JSON file uses fixed-length (tuple-like) arrays in some places. To make it easier to read, I suggest replacing these with objects (
{}) with descriptive property names.
I will take a closer look at specific parts of the code and go into more detail in a later review, once CI is green.
|
Most of the system reworked, deleted .DS_Store, add some descriptions to the |
|
Sorry there was some typos in |
cjohnson57
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some pretty major (but easily fixable) issues with the setting definition itself, adding this separately before I review the rest of it.
|
Got different errors when trying to generate in English and Japanese. English: Seems to be because the dungeon name has a control code in it. Also, pretty sure the Japanese: Adding def from_bytes(
cls,
bytes: Iterable[SupportsIndex] | SupportsBytes | ReadableBuffer,
byteorder: Literal["little", "big"] = "big",
*,
signed: bool = False,
)@flagrama any ideas here? Anyway, fixing that issue ran into the same issue English did with the dungeon name. |
LangDev here, this is probably because of the given value is For the requested changes, moving the |
cjohnson57
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's my review of the rest of the code. Admittedly mostly nitpicks, but a few actual issues.
Thanks so much for all your work on this!
|
Thanks for your quick fixes on my first two messages! Can confirm they're resolved. I could launch the ROM and talk to NPCs and read hints in both English and Japanese. |
Regarding the last request, I'll edit them right now! |
|
One question, what exactly is the purpose of the |
FYI, the default value is just a change made in python 3.11. If you are using 3.11 or newer, this error won't appear. If you are using anything older this error comes up. If you are using 3.11 or newer but you have your dev environment set up to ensure everything works as far back as 3.8 it may be doing something additional to cause that error to show up. |
|
Interesting, ty for the info! |
Also, the |
|
Gotcha. Should that be "plain" texts? |
Oh, sorry yes |
cjohnson57
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for all your changes! It's good now as far as I can tell.
Before merging, we should also get approval from Fenhl, and probably more testing of full JP seeds.
|
@cjohnson57 Thanks for the detailed reviews! |
|
Add UnitTest that checks language file / properties Below this is template for the errors If the language file misses If some properties are missing / wrong in If some bin files are not included in the |
|
Unless the conflicts revolve with in-game messages or custom texts, all conflicts will be resolved after merging |
|
Reopen this with newer commits |
|
@cjohnson57 @fenhl I think it's ready for the review |
|
Thank you. Once fenhl gets back and does their review, I'll also go through it again, then we'll try to do some testing and then merge. |
|
Currently, the branch conflicts revolving around ASM and generated data |
Yes, those are nothing to worry about. |
|
FYI, fenhl is preparing a PR to your branch for some hint region cleanup. In the meantime I will try to do a test seed with JP full randotext next week to see if anything breaks. |
|
FYI, tried to generate a seed on this branch using Japanese and got this error. The error did not occur when selecting English. Using this settings string: Changing the randotext setting did not help, but setting gossip stones to "no hints" did. Clearer hints or not did not affect it. Most hint distros did not help, however, setting it to "Useless" did fix the issue, which makes sense. Strangely, the error also did not occur with hints set to "Very Strong" or "Very Strong with Magic", but it did when set to "Strong", which would probably make sense if I had more intimate knowledge of the different hint distros. Hopefully, the PR fenhl is preparing to address hints will resolve this issue. |
|
OoT_A0FAE_GJAJIMJB4W_Spoiler.json I started this seed, playing on PJ64, and got an immediate freeze when talking to the nearest NPC (Ingo) Talking to other NPCs and reading signs isn't crashing, so I guess it's something specific to Ingo's textbox. Unfortunately I'm not sure how to check what randotext was assigned to him. |
Ok, this looks like the ones got shuffled didn't get the control code that was supposed to be attached (the "00" + glitch also comes from the same instance) I'll use direct text output to see what caused those |
… before Goto ending to prevent freezes, replace '##' to '#' in hint texts
|
@cjohnson57 @fenhl As far as I tested, the error was caused by 2 factors:
|
|
Sorry there was another glitch revolving I'll fix it right away |
|
@cjohnson57 @fenhl Fixed, changing |
|
Btw, there's some code from @GSKirox that should be helpful for testing this PR, that adds a display on the screen for the current textbox ID. He's going to include it in a later PR that expands debug mode, but for now we can add it, then maybe just undo that commit before merging... If you add the asm hook and C function for |
Could you give me the setting seed / generation seed?
I searched through the file and found out that in JP rom, 060B and 016F has control codes as well |
|
…Map texts, unique id texts and reorganize Message.transform
|
@cjohnson57 All of the presented issues are fixed as far as I know |














Hi, it's been a long time
This PR is redo of all of the functions I made almost 5 years ago
Description
With this PR, you can play OOTR with different languages or event texts / custom texts as well
Addition
Files
Also, you can add another language file just by drag and drop the file with same structure
For how to create and what file is included, checkout
language / README.mdAdditional Control Codes for wide characters
For the character to use, check out
Messages.pyText alignment
How to Change the language?
You can change the language by changing
Main Rules >> Language >> Language SelectionTips for language creation
If you want to add language with characters not included in vanilla game (for example Spanish has
ñ), do the same thing as other images: decompile the original game, replace some files with new one and recompile / get diff withget_diff.pyNotes
Currently, besides English which is already implemented from beginning, it allows you to play with Japanese as well
Why Japanese? well, it's because Japanese is the other language that NTSC rom has and one of language that uses wide characters instead and also because I'm Japanese
Just because I didn't implemented other languages such as German, French or Chinese (which is PAL and iQue version has), that doesn't mean you can't play with those languages or other languages that only got the translated version by using mod / patch or even that never got the version
Not only that, you can create version for events, competition or even speedruns as well (like shortening all of the texts, and speed up everything)
Screen shots