WohlSoft / LunaLua

LunaLua - LunaDLL with Lua, is a free extension mod for SMBX 1.3 game engine, core of the X2 project.
https://codehaus.moe/
GNU General Public License v3.0
33 stars 12 forks source link

Powerup id Extension (Hitbox arrays extension) #32

Closed Core-LOVE closed 1 year ago

Core-LOVE commented 2 years ago

Tried to look into all this stuff, hopefully it works

Wohlstand commented 2 years ago

This isn't enough and possibly will crash: You had added 78 addresses for the player height array, but there are much more used references in the code (totally 159 usages of them)

Core-LOVE commented 2 years ago

This isn't enough and possibly will crash: You had added 78 addresses for the player height array, but there are much more used references in the code (totally 159 usages of them)

Some of them don't contain anything related to old max limit (7), so i skipped over them. That's why not everything is in here

Supermario1313 commented 2 years ago

Out of the 178 occurrences of PlayerHeight in the legacy SMBX code, about half of them have a compile-time constant for the second index. Therefore, having 78 patches for the player height array seems reasonable. However, extra care should be taken since PlayerHeight is a two-dimensional array. Since you're patching the second index, changing the value of the elementBytes argument from the size of an array element (in this case, 2) to the size of an array element times the length of the first index (in that case, 2*5 = 10) should be sufficient.

Supermario1313 commented 2 years ago

By looking at the dissasembly, it appears that, unlike other arrays like the block array, the NPC array or the player array, the player width/height arrays aren't represented in the assembly code by a field containing an address pointing towards the first array element, but rather by a constant address which is present multiple times in the code. For example, 0xb2c6fc doesn't contains a pointer towards PlayerHeight(1, 1) but rather contains directly the value of PlayerHeight(1, 1). Therefore, you'll have to find all the occurences of this address and pass them in the ptrsList parameter of the ResizableGameArray constructor.

TL;DR: Hitbox_h_ptr isn't a short** but a short*.

Bluenaxela commented 2 years ago

This is certainly a useful start, though it's some more ways to go.

As Supermario1313 noted, need to overwrite the address 0xb2c6fc for instance in the instructions referencing that address, so using the ptrsList argument of ResizableGameArray instead of ptr. I gave some more detailed notes about this on Discord.

As they also note, the size per element (elementBytes) constructor argument should be written as 2*5 instead of 2 due do how the other array axis to be allocated is 5 indices wide.

In addition, I notice that the patch addresses listed such as 0x8C0354 appear to point to the start of the instruction in question, when what is required is pointing to the byte in the instruction that contains the number that needs to be updated. The ResizableGameArray class currently only handles patching 32-bit limit numbers, since that's all that's been required up till now, however I think mos if not all of the comparison instructions for this '7' limit use a single byte for the limit.

Address 0x8C0354 is a good example of both, as it is the instruction cmp edi,7 represented by the bytes 83FF 07 and it is the 07 that need to be overwritten, so the address should be 0x8C0354+2 instead of just 0x8C0354. It also shows the number to update as being just a single byte in a short instruction, so it would be necessary to do something such as update ResizableGameArray to have an extra constructor argument to flag if the size of the patches should be treated as uint8_t* instead of the default uint32_t*.

One more step to complete this as well, if you want to go beyond 10 states (not of just from 7 to 10), is the arrays for graphics pointers, see these: Defines.h:460 SMBXImageCategories.h:89 You'll notice the NPC/Block/BGO ID extensions do extend to corresponding GM_GFX and IPictureDisp arrays that are relevant. Gladly there are not many references to each of these arrays. Could also do initial testing with 10 states without doing this part though, and focus on getting the other parts sorted out.

(This is much more minor, but I also notice that the pull request's commit also accidentally undoes the change Wohl made to LevelCodes/Docopoper-Calleoca.cpp 10 days ago)

Supermario1313 commented 2 years ago

I already discussed with Core in dm about the points you raised in your 3rd and 4th paragraphs, that's why I haven't left a message here about them.