-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Provided a DEFAULT_PHYSICS value #14767
Conversation
Hi, Zughy wants a new API feature with this available. Not a C++ static |
You mean function to restore this value? Please help me with more details, or guides, I'm actually a newbie in this project |
Something like |
Is it okay to put this in remoteplayer.h or player.h to implement them? If so I'll go with it |
I have basically zero knowledge of the engine structure itself, I'll let a dev answer |
f9a201e
to
082a967
Compare
I see no reason why this can't just be a relatively trivial addition of a table of defaults in |
Acknowledged. I'll do it |
082a967
to
4c2bb1d
Compare
I think that having a single table with all the values inside is better than having all the separate single values |
I agree |
Hey Zughy, I wanna know is it looks good to you now except for this? If there're more problems with the last commit, I wanna solve them in next commit, together with this one |
I guess (can't test) |
I moved those constant in a table, and after thinking I kept changes in constants.h, for |
|
||
-- DEFAULT_PHYSICS constants of a player | ||
core.DEFAULT_PHYSICS = { | ||
["PLAYER_SPEED_DEFAULT"] =1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
["PLAYER_SPEED_DEFAULT"] =1, | |
PLAYER_SPEED_DEFAULT =1, |
There's no need to complicate the declaration with brackets and quotes. Same for all the other fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also consider the formatting a bit weird. There isn't really a benefit to having these "align". Just do speed = 1,
etc.
|
||
-- DEFAULT_PHYSICS constants of a player | ||
core.DEFAULT_PHYSICS = { | ||
["PLAYER_SPEED_DEFAULT"] =1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, you have to use the same field names required by set_physics_override()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I'll check out later
Actually, I'm not a core dev so I have no idea of how the C++ part works. I'm generally pretty confused of the existence of the constants.h duplication (breath and hp as well), so I'll leave core devs answer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be documented in doc/lua_api.md
.
@@ -97,6 +97,28 @@ with this program; if not, write to the Free Software Foundation, Inc., | |||
// Default maximal breath of a player | |||
#define PLAYER_MAX_BREATH_DEFAULT 10 | |||
|
|||
|
|||
// DEFAULT_PHYSICS constants of a player |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These C++ changes should be removed; this is dead code that doesn't have any effect on anything at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean these definations such as #define PLAYER_SPEED_DEFAULT
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. These definitions are unused.
|
||
-- DEFAULT_PHYSICS constants of a player | ||
core.DEFAULT_PHYSICS = { | ||
["PLAYER_SPEED_DEFAULT"] =1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also consider the formatting a bit weird. There isn't really a benefit to having these "align". Just do speed = 1,
etc.
Goal of the PR
As the title said
How does the PR work?
Struct
PlayerPhysicsOverride
already has its own default value, same as what's in API, don't need a new type declaration.So I just add one static variable toremoteplayer.h
.Does it resolve any reported issue?
Closes provide a
minetest.DEFAULT_PHYSICS
value #14764This PR is Ready for Review.
How to test
Check the changed src.