search

UsernameFodder / pmdsky-debug

Debug info for reverse engineering PMD: Explorers of Sky
GNU General Public License v3.0
38 stars 20 forks source link

Touchscreen struct and script parameter conversion functions #254

Closed Frostbyte0x70 closed 4 months ago

Frostbyte0x70 commented 4 months ago

This PR contains two minor additions. First, I've documented a struct that can be used to read data about the current status of the touchscreen, including whether it's currently being pressed or not and the position being pressed. This data, unlike the raw values that can be read from the I/O ports, is expressed in pixels starting from the top left corner of the screen, which should be a better spot to read touchscreen coordinates since it's easier to use and consistent across emulators (seems like the raw values are not). Second, I've researched the function previously known as ProcessScriptParam and its sister function. We now have a better understanding of how scripting opcode parameters work, and how the two special bits they contain affect their value.

A couple of questions:

  1. Should I document those two special bits on scripting opcode parameters? Should a new type be created for those parameters? It's just a halfword composed of 2 bit flags + a 14-bit number.
  2. I don't know if there's a better way of documenting the type returned by ScriptParamToDecimal. I'm not sure what name I could give it, nor how to explain it in a clear and concise way. It's just a number that's stored multiplied by 256, should we add a new type for that?
UsernameFodder commented 4 months ago
  1. Should I document those two special bits on scripting opcode parameters? Should a new type be created for those parameters? It's just a halfword composed of 2 bit flags + a 14-bit number.

We could make a bitfield, but I think the documentation in the function description is enough. I'd be fine leaving these as uint16_t.

  1. I don't know if there's a better way of documenting the type returned by ScriptParamToDecimal. I'm not sure what name I could give it, nor how to explain it in a clear and concise way. It's just a number that's stored multiplied by 256, should we add a new type for that?

As I mentioned, these are fixed-point numbers. For the 64-bit variants, there actually is a type, struct fx64, which is used heavily in the damage calculation routines. But this is mainly because it was wider than the native 32-bit register size. For the 32-bit variants, I never bothered and just pass them around as int32_t or uint32_t. I think it'd be reasonable to leave the ones here as int16_t, since we'll be documenting them in the symbol descriptions.

Frostbyte0x70 commented 4 months ago

Alright, everything is updated and fixed. I also rebased master since there were new commits from Marius' PR. Let me know if it looks good now or if something still needs to be changed. I did not create new types for the fixed-point numbers, since it seems like describing them is enough.