lushen124 / Universal-FE-Randomizer

Properly universal this time.
MIT License
98 stars 28 forks source link

Small Issues Character Shuffling #415

Closed Geeene closed 8 months ago

Geeene commented 1 year ago

I wanted to try generating a rom with a custom cast for a playthrough that I want to do, and while working on this, I noticed some small issues with the Character Shuffling.

Issue 1: 2023-04-07 19_53_29-Yune_Trunk - Universal FE Randomizer_src_random_gba_randomizer_shuffling_Charact

As the Weapon Ranks were transfered 1-to-1 (just scaled) it didn't actually check that the class of the unit was still able to use all the weapon ranks.

We should make sure that if classRank == 0, then we don't set the rank.

Issue 2: My custom cast contained chars from all three games.

When randomizing I sometimes noticed that some names were incorrect. After some quick analysis, the reason here seems to be as following:

Shuffle in a character that still exists in the current game (i.e. you have the character twice), then replace the original version of the character. The text replacement has no way of knowing which of the texts are the shuffled in and which are the original character.

I'm not yet sure if / how this could be fixed.

I'll also add the program I used to generate the custom cast config from a txt of names. The same function could also be used inside Yune technically, would be a question of how we would want to do it.

lushen124 commented 1 year ago

As the Weapon Ranks were transfered 1-to-1 (just scaled) it didn't actually check that the class of the unit was still able to use all the weapon ranks.

Yeah, there's some logic in randomized recruitment logic that does this transfer. I don't remember if this is the exact logic, but it does something like: take the character's weapon ranks, and map weapon ranks to the target class starting with the highest rank, either going down the list or repeating the rank for each weapon type in the target class.

For example, if you're transferring FE7 Marcus, he starts with rank A, D, and E (the actual types don't matter). If he becomes a Swordmaster, he gets A rank swords. If he becomes a Hero, he gets either A rank Swords and D rank axes, or vice versa. If FE7 Karel (who starts with A rank) becomes a Hero, he'd get A rank swords and axes.

Shuffle in a character that still exists in the current game (i.e. you have the character twice), then replace the original version of the character. The text replacement has no way of knowing which of the texts are the shuffled in and which are the original character.

The original logic in recruitment randomization logic handles this because it collects all of the changes necessary first and then uses Matcher to make sure everything is only updated once.

For example, if you're randomizing FE7 and you re-import Eliwood as Hector's replacement and then import, say, Roy as Eliwood. The logic should generate the regex string:

\b[H-]*Hector\b|\b[E-]*Eliwood\b

A real string would link together every name it wants to replace (and the all-caps versions) and be MUCH longer. It then runs through every string and processes it with the Matcher. Matcher processes each regex match once and performs the replacement, so if your string is:

"Hector, you and Eliwood were right!"

Matcher.find() will find "Hector" first and replaces it with "Eliwood". Matcher.find() then proceeds to process "Eliwood" and replaces it with "Roy". At that point, calling Matcher.find() should return false, so it should finish processing that string and move to the next one.

The final string would be:

"Eliwood, you and Roy were right!"

The important part is to make sure the replacement table is complete first before you do any replacing, because the goal is to process each string exactly once.

Geeene commented 1 year ago

Ah I see now, makes sense.

The issue was just that the old logic from before the pattern replacement was still there, (+ applying the changes during rather than after the loop).

That's easy enough to fix. Thanks :)

Geeene commented 1 year ago
  1. Leaving Descriptions unchanged setting doesn't work properly: 2023-04-08 16_47_04-Yune_Trunk - Universal FE Randomizer_src_ui_CharacterShufflingView java - Eclips

Initialization of the option is just wrong, so right now you can't have them be left as they were.

Geeene commented 1 year ago

Incorrect Blinking frame coordinates: Melady, Dayan, Rutger, Zelot

Incorrect Mouth Frames: Fae, Wendy, Dorothy

I'll just double check all of the FE6 Frame coordinates ones..

(in FE8) Ogier / Oswin / Ostia have a space between the capital o and the second letter, pretty sure this is about the font table and might be hard to fix?

lushen124 commented 1 year ago

I'm not sure why it'd have a space unless the font just does that if you have a word that begins with a capital O. And if that's the case as you suspect, then yeah, probably not worth fixing.

Geeene commented 1 year ago

2023-04-09 23_52_54-Window

I also found that there are more texts that should be excluded from text replacement. Though I'm not actually so entirely sure that every case can just be excluded the way I currently do it f.e. 0x0B74 / 0x0B73 which are the converstaion between your lord and Innes when he got Vidofnir / Nidhogg, where he calls Vidofnir the "Winged Lance", lances name is just so unfortunate lol.

Do you still remember if Random Recruitment text replacement made issues with lance in FE6? and if so how you addressed that?

lushen124 commented 1 year ago

It probably doesn't handle that case very well either. I'm pretty sure I just made sure text within certain indices were just not modified by name changes, specifically the list of item names, since I think they're all close together.

But then again, it should only be FE6 that has this issue, because that's the only place where existing dialogue with "Lance" might need to be replaced with a different name. If you're running random recruitment AND character shuffling, then it might be enough to just run randomized recruitment first before character shuffling. It certainly solves the FE8 issue at least, but I don't know if FE6 has the issue as I never extensively tested it. I think the game uses "lance" (lowercase L) in a few places, but we specifically look for proper names with capital L, so we shouldn't be picking those up in the text replacement. I don't know if we do this already, but if not, that would be the simplest fix.

Geeene commented 1 year ago

that is certainly an option, I wanted to do Shuffling first, but I guess the order that the shuffled characters join in is random anyway.

Maybe with not having the Autoleveling logic from Random Recruitment being applied to the shuffled characters anymore it might be worth to overtake it rather than doing simple autoleveling with the configured growths / bases (atleast as an option).

Also you are right about the capital Lance vs lance issue, and this didn't appear in fe6 as in the currently used translation patch it doesn't use capital Lance (for items) 2023-04-11 21_41_38-Window

So I certainly think you are right, we should just replace Lance with lance right at the start before any other text replacement activity.

lushen124 commented 1 year ago

If you want to retain the order, then the other option is to defer applying the name swaps until the very end. The important thing is that it's done only once, not necessarily which step you do it on. You can still build the regex string and the name mapping up front, but just don't apply it until you've done both steps (and any other steps that require text replacement).

The regex string is actually fixed per base game, since it only lists the names that need replacing. So FE6's regex string will only ever have the FE6 characters' names, for example.

Geeene commented 1 year ago

I do feel good about just moving the shuffling after the random recruitment. I do forsee it maybe making sense to overtake the growths / autoleveling settings (fill, slot, slot relative) etc. to this feature, but I haev nothing to back that up, so not doing that until I get some more opinions

lushen124 commented 8 months ago

I've made some minor changes to get ready to merge with the other PRs I've already merged. Let me know if there's more you want to do to this. I'll leave it open for another week or so.

Geeene commented 8 months ago

Nope, everything I had changed on my forks master looks to have been added to this PR aswell :) If there is more changes I would want to do in the future I will just open a new PR for those