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

Use s64 instead of s16 for actor parameters #126

Closed
wants to merge 1 commit into from

Conversation

Yanis002
Copy link
Collaborator

this was an idea I had after talking with @Reonu about how actor parameters works last month

I kept the pointers because I wanted to make sure it worked fine but imo they're all completely useless, also I used a signed long long instead of an unsigned because it was raising warnings, I compared with current develop/2.1.0 and I have the same number of warnings now so everything should be fine hopefully (ngl I didn't tested this extensively but overall it seems to work properly)

so with this change, if I'm not dumb, instead of having parameters on 0xABCD, we can have parameters on 0xABCDEFGHIJKLMNOP

@Thar0
Copy link
Contributor

Thar0 commented May 31, 2024

Increasing the width of the params field is not so easy as just changing some types unfortunately. Some actors depend on 16-bit sign extension for correct behavior (look for just about anywhere that checks params < 0). Further, 64-bit types do not compile to particularly nice code under the O32 ABI; 32-bit params are surely sufficient for most situations, with the usual z-rotation fallbacks in particularly pathological cases?

I think this needs some careful discussion, it's a topic that doesn't seem like it should be complex but I think it may be.

@Yanis002 Yanis002 marked this pull request as draft May 31, 2024 01:32
@Yanis002
Copy link
Collaborator Author

ok well, I made a it a draft for now I guess

@Yanis002
Copy link
Collaborator Author

Yanis002 commented May 31, 2024

32-bit params are surely sufficient for most situations, with the usual z-rotation fallbacks in particularly pathological cases?

tbh I'm always really frustrated by s16s when it comes to parameters, I like to add tons of features to the actors I make or edit so it gets full quickly and I have to make sacrifice or use the rotations, which I find weird btw

one solution would be using a secondary parameter variable, I assume this would solve most concerns here? not sure how it would affect compilation though (thinking about ActorEntry

@Reonu
Copy link
Contributor

Reonu commented May 31, 2024

32-bit params are surely sufficient for most situations, with the usual z-rotation fallbacks in particularly pathological cases?

tbh I'm always really frustrated by s16s when it comes to parameters, I like to add tons of features to the actors I make or edit so it gets full quickly and I have to make sacrifice or use the rotations, which I find weird btw

one solution would be using a secondary parameter variable, I assume this would solve most concerns here? not sure how it would affect compilation though (thinking about ActorEntry

having a second variable sounds way more reasonable and safe to me, it's less likely to break vanilla behavior and it makes it easy to nicely separate vanilla behavior from custom behavior when editing vanilla actors

@Yanis002
Copy link
Collaborator Author

closing for now

@Yanis002 Yanis002 closed this Dec 31, 2024
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.

3 participants