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

Allow literal '0' as float parameter when calling opcodes #77

Merged
merged 5 commits into from
Feb 28, 2024

Conversation

MiranDMC
Copy link
Collaborator

No description provided.

@MiranDMC MiranDMC requested a review from x87 February 27, 2024 21:26

// people tend to use '0' instead '0.0' when providing literal float params in scripts
// binary these are equal, so can be allowed
if (var.dwParam == 0)
Copy link

Choose a reason for hiding this comment

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

0x3f800000 is a valid float (1.0). Can I use it in my scripts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is already existing problem as valid until today scripts throws critical errors because someone did
setPos 0 0 0 instead of setPos 0.0 0.0 0.0. Sanny did not carred and compiled it as BYTE instead of FLOAT.

I have not yet met case where somebody did setPos 0x3f800000 0x3f800000 0x3f800000

Copy link

Choose a reason for hiding this comment

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

my suggestion would be to allow any int values in place of floats. values like 0 or 0x3f800000 can work out perfectly fine.

the opposite should be disallowed. using floats in place of integers is almost always a mistake (maybe except 0.0, however is_player_playing 0.0 makes little sense)

Copy link
Collaborator Author

@MiranDMC MiranDMC Feb 27, 2024

Choose a reason for hiding this comment

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

1 is not binary equal to the 1.0 so it is actual mistake
Yep, Sanny still allows it, ignoring the type defined in json.

Copy link

Choose a reason for hiding this comment

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

I dunno, I don't see a point in made-up limitations. If someone wants to write setPos 0x3f800000 0x3f800000 0x3f800000 what's bad about it? It is valid and working code, why should we limit it?

Copy link
Collaborator Author

@MiranDMC MiranDMC Feb 27, 2024

Choose a reason for hiding this comment

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

Nobody writes that. But usually people write setPos 1.0 1.0 1 which results in float like 1.4E-50

In my opinion SB should automatically convert 1 into 1.0 during compilation if opcode expect float parameter.

Copy link

@x87 x87 Feb 28, 2024

Choose a reason for hiding this comment

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

We can't use "nobody writes that" argument. there could be an infinite number of scripts and some of them may use such code for some reason.

I agree that 1 is likely an overlook the user should be warned about. However, we also reject whole class of valid values which work with original opcodes, but not with CLEO ones.

at some point there should be a switch to disable some or all these restrictions. Possibly something like debug/release builds.

@MiranDMC MiranDMC merged commit e6ab655 into master Feb 28, 2024
@MiranDMC MiranDMC deleted the read_float_param_tweak branch February 28, 2024 00:47
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