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

Change gravity type to float #832

Merged
merged 2 commits into from
Jan 8, 2024
Merged

Conversation

vedran77
Copy link
Contributor

@vedran77 vedran77 commented Jan 6, 2024

@CLAassistant
Copy link

CLAassistant commented Jan 6, 2024

CLA assistant check
All committers have signed the CLA.

@shierru
Copy link

shierru commented Jan 6, 2024

The fact that the data type is wrong is correct, but the fact that you fixed it with a number (0.008) is wrong. I already mentioned in the Issue that there is a parameter game.gravity in config.json and it should be the default value.

I think the main problem lies in the data type, which was set incorrectly, because of which the GetPlayerGravity() function returned an incorrect value .

@vedran77
Copy link
Contributor Author

vedran77 commented Jan 6, 2024

Thank you for your feedback. Regarding the instances where I used 0.008f in the code, please note that these values are not intended for actual use; rather, they are included solely for resetting player variables.

The default value (specified in config.json) will be applied each time a player is initialized. You can find the relevant implementation here: link to the code.

@shierru
Copy link

shierru commented Jan 6, 2024

In my opinion it is better to use 0.0 than 0.008, because then 0.008 becomes a "magic number" and it requires an additional declaration of it as a constant.

@vedran77
Copy link
Contributor Author

vedran77 commented Jan 6, 2024

You are probably right; the only reason I set it to 0.008 is because I saw in the documentation that the default value is 0.008. However, upon further consideration, it makes sense for it to be 0.0.

@Y-Less
Copy link
Collaborator

Y-Less commented Jan 6, 2024

@shierru is correct about using the config value. As it stands now calling GetPlayerGravity on a new player will return the wrong value, where that should really be possible.

@vedran77
Copy link
Contributor Author

vedran77 commented Jan 6, 2024

I don't know how to reproduce that issue. Here are the things I did:

  • Created two commands:
YCMD:getgravity(playerid, params[], help) {
    printf("playerid %d - gravity %f", playerid, GetPlayerGravity(playerid));
    return 1;
}

YCMD:setgravity(playerid, params[], help) {
    SetPlayerGravity(playerid, 0.044);
    return 1;
}
  • Set gravity to 0.01 in my config.json.
  • When the player with ID 0 initially connected, they received gravity set to 0.01.
  • Then, ID 0 executed the command /setgravity and got gravity set to 0.044.
  • After that, ID 1 joined the server and executed the command /getgravity, but the result was 0.01.

What did I miss?

@Hual
Copy link
Collaborator

Hual commented Jan 8, 2024

You didn't miss anything - the gravity is already set to the config one in onPeerConnect()

@Hual Hual merged commit fc5b4c8 into openmultiplayer:master Jan 8, 2024
1 check 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.

5 participants