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

Player API: Animation-dependent player model collisionbox #2742

Closed
appgurueu opened this issue Jun 30, 2018 · 23 comments
Closed

Player API: Animation-dependent player model collisionbox #2742

appgurueu opened this issue Jun 30, 2018 · 23 comments

Comments

@appgurueu
Copy link
Contributor

Issue type
  • Feature request
Minetest version

Any I think

Summary

When a player dies, he leaves a dead body, which AFAIK only disappears when this player hits the respawn button. And this dead body has bad collision box & prevents for instance firearms from shooting through. IMO, dead body should disappear after 5 secs or so, and not only when respawn button is pressed. I know that might pose a problem, as the player entity has to be somewhere even while he's dead, but why not temporarily remove model & collisionbox, make them invisible ?

I'm pretty sure I can't change that outta a mod...

@SmallJoker
Copy link
Member

https://github.com/minetest/minetest/blob/f3b7be97feb5f1de8f31c1841cb7fdd3d89ac451/src/content_cao.cpp#L345-L346
Extend with || m_hp == 0 and
https://github.com/minetest/minetest/blob/f3b7be97feb5f1de8f31c1841cb7fdd3d89ac451/src/content_sao.cpp#L821
with || m_hp == 0

Easy PR. Makes the selection box not selectable for dead objects but keeps the model to indicate that the player died (nametag will also be there).

@appgurueu
Copy link
Contributor Author

What does the Beginner Friendly label mean ?

@SmallJoker
Copy link
Member

It means that it's friendly to beginners.

@appgurueu
Copy link
Contributor Author

In the game or in implementation ?

@paramat
Copy link
Contributor

paramat commented Jun 30, 2018

I assume you mean 'bad collision box' as in an upright box for a player laying down. And i assume the selection box is the issue not the collision box.
I think the body should remain until 'respawn'. I disagree with removing the selectionbox even if it is the wrong shape, after all the body should be selectable as it is an obstacle.
Instead are we able to give a dead body the correct shape selectionbox (EDIT: and collisionbox)? That seems better, this may be doable in MTG.

@appgurueu
Copy link
Contributor Author

Your assumptions are right.

@appgurueu
Copy link
Contributor Author

if we give the body the correct selectionbox, it should have a correct collisionbox, too. It's strange when selectionbox and collisionbox are different. Plus, you should give a dead body more gravity(=more acceleration downwards). I may open a new feature request/issue : I really want to be able to access player velocity/acceleration etc. from a mod, just like with entities...
Then, if you gave the dead body that acceleration, check whether its speed changes. That means it has reached a solid node. If yes, start a player:move_to, and make him sink slowly downwards. And if the player want's to respawn, replace it with an entity. This entity should have a lifetime of ~20 secs.
How about that ?

@ClobberXD
Copy link

ClobberXD commented Jul 1, 2018

@appgurueu, see the LuaEntitySAO methods section of lua_api.txt. I've provided two of them here for convenience:

@paramat
Copy link
Contributor

paramat commented Jul 1, 2018

it should have a correct collisionbox, too

Agreed, sorry to imply otherwise.

Plus, you should give a dead body more gravity

No, obviously.

There is an issue open about strange behaviour of dead bodies.

And if the player want's to respawn, replace it with an entity. This entity should have a lifetime of ~20 secs.

This is unclear, replace the dead body with an entity after player respawn?

@appgurueu
Copy link
Contributor Author

@paramat : Okay, but leave it with normal grav, at least. "replacing with entity" - Yeah so the dead body doesn't disappear before it's lifetime ended. Could be done from a Mod.
@ClobberXD : Afaik you are wrong, as player extends it, yes, but in MT 0.4.16.1 I can't access those methods...

@paramat
Copy link
Contributor

paramat commented Jul 1, 2018

The body disappears when the player respawns because that is when the body comes back to life, otherwise would be unjustified complexity. In MTG that is when bones are placed.

Dev discussion is about latest master, not 0.4.16.

@paramat
Copy link
Contributor

paramat commented Jul 2, 2018

👎 for request but giving a dead body the correct boxes may be a good idea.

@paramat
Copy link
Contributor

paramat commented Jul 24, 2018

PR #2184

@paramat paramat changed the title Player respawning Player respawning (problematic dead player selection/collision boxes) Nov 17, 2018
@An0n3m0us
Copy link
Contributor

What about a tomb stone? I set the character to another texture on hp = 0:
test1
test2

@ClobberXD
Copy link

What's left of this PR now? Hasn't this been decided to be resolved in MTG?

@paramat
Copy link
Contributor

paramat commented Sep 17, 2019

See #2184 (comment) onwards, needs consideration.

@ClobberXD
Copy link

Right, thanks.

SmallJoker's idea is quite interesting. It's unfortunate you've decided to drop the PR. :)

@paramat
Copy link
Contributor

paramat commented Sep 22, 2020

I have been thinking about this recently.
The underlying problem here is that the differing animations of the player model all have the same collisionbox (and eye height).
This is a significant issue and can be solved with alteration to the player API, i think SmallJoker's suggestion in #2184 is going in the right direction. I might work on this.
So i am transferring this to MTG.

@paramat paramat transferred this issue from minetest/minetest Sep 22, 2020
@paramat paramat changed the title Player respawning (problematic dead player selection/collision boxes) Problematic dead player selection/collision boxes: All animations of a model have the same collisionbox (and eye height) Sep 22, 2020
@paramat paramat self-assigned this Sep 22, 2020
@paramat
Copy link
Contributor

paramat commented Sep 23, 2020

Obviously, some animations of a player model are likely to require a different collisionbox and eye_height, for example when sitting or laying down. But the Player API is designed to have a single collisionbox and eye_height for all animations.
What i am thinking of is using SmallJoker's approach explained in #2184 (comment) , with collisionbox and eye_height defined as tables with values for each animation name.

It needs to be backwards compatible with registered models that do not have tables for collisionbox and eye_height. So considering collisionbox as an example i suggest this as an initial idea:

  • In player_api.set_animation(), the collisionbox field is tested for being a table or not.
  • If a table, set player collisionbox to the collisionbox determined by the animation being set.
  • If not a table (backwards compatibility), set player collisionbox to the model's single specified collisionbox.

Can anyone see any problems with this approach?
I am not sure if stepheight needs to alter per animation, but it could do with some uses.
player_api.set_animation() can already alter animation speed.

@paramat paramat changed the title Problematic dead player selection/collision boxes: All animations of a model have the same collisionbox (and eye height) Player API: All animations of a player model have the same collisionbox (and eye height) Sep 23, 2020
@appgurueu
Copy link
Contributor Author

Obviously, some animations of a player model are likely to require a different collisionbox and eye_height, for example when sitting or laying down. But the Player API is designed to have a single collisionbox and eye_height for all animations.
What i am thinking of is using SmallJoker's approach explained in #2184 (comment) , with collisionbox and eye_height defined as tables with values for each animation name.

It needs to be backwards compatible with registered models that do not have tables for collisionbox and eye_height. So considering collisionbox as an example i suggest this as an initial idea:

In player_api.set_animation(), the collisionbox field is tested for being a table or not.
If a table, set player collisionbox to the collisionbox determined by the animation being set.
If not a table (backwards compatibility or no need for differing collisionboxes), do nothing and so continue to use the model's single specified collisionbox.

Can anyone see any problems with this approach?
I am not sure if stepheight needs to alter per animation, but it could do with some uses.
player_api.set_animation() can already alter animation speed.

That is the straightforward approach (except I would just check for falsy instead of checking for type)

@paramat paramat changed the title Player API: All animations of a player model have the same collisionbox (and eye height) Player API: Animation-dependent player model collisionbox Sep 25, 2020
@paramat paramat removed their assignment Oct 2, 2020
@ghost
Copy link

ghost commented Oct 6, 2021

Things to consider:
Mounting to eg. vehicle. #2769

Mods: Player classes (short, tall)
Mods: Entity Modifier resizeme x40 or x0.5

@appgurueu
Copy link
Contributor Author

Things to consider: Mounting to eg. vehicle. #2769

Engine issue, not related to this PR: The boat collides with the environment, the attached player does not. The collisionbox only matters for external objects colliding with the player. This PR will improve upon that as a sitting player should have a different collisionbox than a standing one.

Mods: Player classes (short, tall) Mods: Entity Modifier resizeme x40 or x0.5

Out of the scope of this PR. These mods need to see how to not mess with other mods (including player_api) by overriding player properties.

@appgurueu
Copy link
Contributor Author

Closed by #2745

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants