Reworked TMHM into expandable list format

[gruxor]

Description

Completely removed the need for bit arrays, instead using the format of gLevelUpLearnsets as a guide to rework TMs/HMs entirely. TMHM lists use MOVE_UNAVAILABLE as a footer to allow proper looping, since dynamic arrays can't have their sizes determined at compile time. gLevelUpLearnsets uses an identical method through LEVEL_UP_END which shares the same value (max size u16, i.e. 0xFFFF). Optionally, a new preprocessor define could be used instead for the footer to prevent issues with any future integer type modifications.

• Should be much friendlier for new faces and less of a headache for old faces, with virtually no difference in performance/memory that I'm aware of.
• Should cover all use cases - functions that the data was/is fed into have all been accounted for across the entire project (there weren't many)
• Haven't yet touched the tutor movelist (which uses an almost identical bit array system) but I imagine someone wanting to expand that list is much more rare. Could be easily done in the future if necessary, or just for consistency/ease of use
• Movelists may have minor errors since I regex'd the old list during conversion, but I'm pretty sure the compiler already caught most if not all of those during the testing process
• Support for PoryMoves should be a fairly trivial modification since the app already supports leveled lists
• Probably conflicts with Implemented support for 128 bit tmhm learnsets. #2232 (sorry Stephen) as it's an alternative solution to the same problem, but without the 128-bit limitations and with added user-friendliness

Discord contact info

Crater#7777

[StephenLynx]

You skipped the use of P_NEW_POKEMON to enable some entries?

[gruxor]

You skipped the use of P_NEW_POKEMON to enable some entries?

Checks for P_NEW_POKEMON still exist in tmhm_learnset_pointers.h, which determines if the arrays are actually used. They're statically defined then added to gTMHMLearnsets and can't really be used without it (and it'd defeat the purpose to access them directly anyways).

Preprocessor if statements could be introduced (re-introduced?) to tmhm_learnets.h, but I doubt it really makes much of a difference in the end.

edit: just checked, only 2 exist in level_up_learnsets.h, so moving those over would be super trivial if they're wanted for reasons that I'm missing or just for consistency's sake

[AsparagusEduardo]

P_NEW_POKEMON is meant to save space, and therefore, any data related to the new mon shouldn't be compiled into the rom if it's not gonna be used.

[gruxor]

Fixed.

[StephenLynx]

Speaking of space, I think it's important to keep in mind this will at least double the space used for this, maybe even triple.

[gruxor]

The space taken up is pretty negligible. It's certainly more than the existing table and even the 128-bit expansion, but it's still incredibly tiny.

The current master branch of the expansion, even with nothing disabled (all the cries, graphics, etc.) and with this change added, takes up only about 23.8MB of the available 32MB in the ROM.

[AsparagusEduardo]

2 additional things:

• Spaces instead of tabs in tmhm_learnsets.h

• Something that I've been thinking is that in vanilla, TMs/HMs and Tutor Moves don't overlap in terms of moves that can be taught. And even if they did, I doubt that they would be able to be learned with one method and not the other. My thought is that we could potentially unify both TM and Tutor tables into a single list, seeing as not only CanMonLearnTMTutor is already used by both, but it would further simplify modifying both Tutors and TMs (eg: no need for TUTOR_MOVE_* and stuff).

[gruxor]

Updated the tabs to spaces, haven't committed it yet pending the following:

Something that I've been thinking is that in vanilla, TMs/HMs and Tutor Moves don't overlap in terms of moves that can be taught. And even if they did, I doubt that they would be able to be learned with one method and not the other. My thought is that we could potentially unify both TM and Tutor tables into a single list, seeing as not only CanMonLearnTMTutor is already used by both, but it would further simplify modifying both Tutors and TMs (eg: no need for TUTOR_MOVE_* and stuff).

I can see that working for most use cases, but not all. For example, if you have one-use TMs (or TRs, if those are implemented) then you could give the player one for a fairly powerful move early on. Then later, maybe you want to grant access to a tutor that can teach it infinitely in exchange for a Star Piece or something. Can't imagine stuff like that would be super common, but it isn't out of the question.

I think it'd definitely still be possible to trim down some of the code if I set up the tutor movelists in the same way, though. I can give it a shot if it'd be better to merge it alongside this change rather than via a separate PR.

[AsparagusEduardo]

I can see that working for most use cases, but not all. For example, if you have one-use TMs (or TRs, if those are implemented) then you could give the player one for a fairly powerful move early on. Then later, maybe you want to grant access to a tutor that can teach it infinitely in exchange for a Star Piece or something. Can't imagine stuff like that would be super common, but it isn't out of the question.

I don't how that couldn't work with what I proposed. If the mon is able to learn the move with a single-use TM, it should also be able to learn that move from that Star Piece Tutor if both instances are read from the same move compatibility table.

[gruxor]

I don't how that couldn't work with what I proposed. If the mon is able to learn the move with a single-use TM, it should also be able to learn that move from that Star Piece Tutor if both instances are read from the same move compatibility table.

That's true, I don't know why I was thinking the method would matter. The thing in question would be "should a species be able to learn a certain move from a TM and not a Tutor, or vice versa?" and I really can't think of any examples where anyone would want that.

I'll see what I can do, but I'll probably have to set up a script of some sort this time since I'll need to merge the arrays.

[AsparagusEduardo]

I'll see what I can do, but I'll probably have to set up a script of some sort this time since I'll need to merge the arrays.

Either that, or we could wait until I add beta PoryMoves support for this 👀. Don't know how long that would take though, I tend to get distracted a lot lol

[gruxor]

Massive changes with ed02c4b. Boom.

• Merged tutor and TMHM learnlists into one
• Removed incredibly stupid arg requirements for the "can this pokemon learn this move" functions - now take in a move ID instead of needing to pass through a dummy item ID or dummy tutor ID based on the usage
• Removed gTutorMoves entirely, tutors now use move IDs directly rather than an abstracted list
• Removed more than a handful redundant carry-overs from the leveled lists. Lists existed for forms that have no differences, such as the Vivillion colors and many others. These were already not being used, because the pointer lists never assigned anything to them, instead using their base form. I'll probably submit a separate PR to clean up the leveled lists in the same way since that's the source of the redundant lists.

I did testing with Treecko to check TMs, HMs, and tutors as well as move-overwriting for each. May need additional testing though. Initially had issues regarding the move being stored in gPartyMenu.data1, but I'm 99% sure I fixed all quirks and bugs on that front already.

[AsparagusEduardo]

Awesome, this is great to see!

Though, there are some things that I feel should be changed name-wise:

• gTeachLearnsets ->gTMTutorLearnsets

• CanMonLearnTaughtMove -> CanMonLearnTMTutorMove

• CanSpeciesLearnTaughtMove -> CanSpeciesLearnTMTutorMove

• CanMonLearnMove -> CanMonLearnTMTutor

• teach_learnset_pointers.h ->tm_tutor_learnset_pointers.h

• teach_learnsets.h -> tm_tutor_learnsets.h

• sSPECIESTeachLearnset -> sSPECIESTMTutorLearnset

[gruxor]

... Though, there are some things that I feel should be changed name-wise: ...

Discussed on Discord, updated in f8fb4e8.

[gruxor]

Rushed the initial changes to Battle Frontier and assumed some variables meant something they didn't actually. Now, having fully deciphered the spaghetti, the tutors should actually be fixed in 9f16d1d.

Gotta love multi-use unnamed variables.

Requested changes were implemented.