Follower female fix

[hedara90]

Description

Fixes followers breaking for females with high species numbers.
Removed vestigial shiny handler not present in the expansion version of the followers branch since it's handled with a bitfield instead.
Updated a graphicsId comment to represent current expansion status.
Fixed this fix breaking followers in the sprite visualizer.

Changed a follower form extraction to use updated expansion values (maybe, I don't know if that function actually does that, hopefully someone who knows more can verify)

Images

itsfixed.mp4

Issue(s) that this PR fixes

Fixes #5470

Feature(s) this PR does NOT handle:

Hisuan Sneasel brokeness in sprite visualizer

Discord contact info

hedara

[AlexOn1ine]

any reason this isn't just removed?

[hedara90]

Showing the previous state of the code.
I'm not sure if this part of the code does what I think it does.

[AlexOn1ine]

Can we not check the previous state through git or this pull request?

[hedara90]

Yes, but it's also the code that's meant to stick out because I'm not sure about it.
It can be removed on Github when someone who knows more agrees that the new line is correct.

[mrgriffin]

Huh. 0x7d is bufferspeciesname, and T1_READ_16(&template->script[2]) corresponds to the species argument.

So I suppose what we're saying here is that if the script starts with bufferspeciesname _, X then form will be stored in the top 5 bits of X.

Ah, there's a comment just above which explains what I said, lol!:

    // set form based on template's script,
    // if first command is bufferspeciesname

I think this change is needed because our species have 12 bits (EDIT: Our species have 11 bits, but OBJ_EVENT_GFX_SPECIES_BITS is 12 bits). I think for sanity's sake form = template->trainerRange_berryTreeId & 0x1F; should probably also use a 0x0F mask because there's only 4 bits available for form in the graphics ID. There should probably be a constant for that mask to make it easier to understand what's going on? OBJ_EVENT_GFX_FORM_MASK or something.

I have to say I'm quite confused about the purpose of this code. Is the idea that maybe you'd have an Unown's graphics ID set to the base form, but then in the script you could specify a specific Unown form for it to be displayed as (I guess there's a macro somewhere that does (species) | ((form) << 10)?) I'd love to know why it can come from either the script or trainerRange_berryTreeId... maybe the latter is useful from Porymap, but you need the former if you want to make this object be a trainer? Is there some reason why we can't just set the graphics ID to the combined species-form constant?

[mrgriffin]

Ah, also the shift in this should be by OBJ_EVENT_GFX_SPECIES_BITS rather than by 10 (again because of our species having 12 bits).

EDIT: This is wrong. We have 11 bits. So a shift by 11. But then still masked to 0x0F because the graphics ID still only has 4 bits for forms.

[hedara90]

Apparently followers can't be shiny with the PR as it is now in the Sprite Visualizer.
I'll look more into resolving that. And whatever the final state of the PackGraphics part turns out to be. Initial short testing didn't show any change between 12 and 11 bits of shifting.

[kittenchilly]

Apparently followers can't be shiny with the PR as it is now in the Sprite Visualizer. I'll look more into resolving that. And whatever the final state of the PackGraphics part turns out to be. Initial short testing didn't show any change between 12 and 11 bits of shifting.

That might have been what the SPECIES_SHINY_TAG was for

[hedara90]

Marking as draft, going to give fixing both overworld and sprite visualizer at the same time.

[hedara90]

Fixed both the Sprite Visualizer and Followers at the same time, as well as Sneasel-H turning into Onix in Visualizer.
Looks horrible, but it works.

[kittenchilly]

If not packing graphics doesn't cause performance or size issues then it should be fine, right?

[hedara90]

As far as I can tell, we weren't actually using that function.

[kittenchilly]

Then it looks fine to me, really.