Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Touchscreen struct and script parameter conversion functions #254

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

Frostbyte0x70
Copy link
Collaborator

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?

@Frostbyte0x70 Frostbyte0x70 force-pushed the minor-changes branch 3 times, most recently from b58155b to 467eb0d Compare April 15, 2024 21:45
symbols/ram.yml Outdated Show resolved Hide resolved
symbols/overlay11.yml Outdated Show resolved Hide resolved
symbols/overlay11.yml Outdated Show resolved Hide resolved
symbols/overlay11.yml Show resolved Hide resolved
symbols/overlay11.yml Outdated Show resolved Hide resolved
symbols/overlay11.yml Outdated Show resolved Hide resolved
symbols/overlay11.yml Outdated Show resolved Hide resolved
@UsernameFodder
Copy link
Owner

UsernameFodder commented Apr 16, 2024

  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
Copy link
Collaborator Author

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.

Copy link
Owner

@UsernameFodder UsernameFodder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good now, thanks!

@UsernameFodder UsernameFodder merged commit b64a9fa into UsernameFodder:master Apr 17, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants