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: Give laying players a low selection/collisionbox #2184

Closed
wants to merge 2 commits into from
Closed

Player_api: Give laying players a low selection/collisionbox #2184

wants to merge 2 commits into from

Conversation

paramat
Copy link
Contributor

@paramat paramat commented Jul 24, 2018

screenshot_20180724_160334

^ Killing player

screenshot_20180724_160354

^ Dead

screenshot_20180724_160435

^ Boxes are sized to diagonally laying player to not be excessively large, quite a nice fit

screenshot_20180724_160502

^ Dead player respawns, boxes reset

Attends to https://github.com/minetest/minetest/issues/7510

mods/player_api/api.lua Outdated Show resolved Hide resolved
@paramat
Copy link
Contributor Author

paramat commented Aug 9, 2018

Updated.

@paramat paramat added the WIP label Aug 10, 2018
Attends to issues caused by dead players having upright boxes.
Add a new model field 'collisionbox_lay'.
Reset boxes to normal on player respawn.
@paramat paramat removed the WIP label Aug 11, 2018
@paramat
Copy link
Contributor Author

paramat commented Aug 11, 2018

Added documentation @SmallJoker

mods/player_api/api.lua Outdated Show resolved Hide resolved
@SmallJoker
Copy link
Member

SmallJoker commented Aug 11, 2018

Regarding different collision boxes. There could be more appearing, depending on the "mode" of the player. For that reason I think it would make more sense to change the format of the collisionbox parameter so that it's like animations. Each "mode" is then indexed by a string key - allowing a new public API function like player_api.set_mode(player, "lay").
EDIT: Or even keep the "animation" name and "collisionbox" name synced. Fall back to the "stand" collisionbox if none is specified for the current animation.

@paramat
Copy link
Contributor Author

paramat commented Aug 11, 2018

Good point, making it expandable for more box types, will try that.

@paramat paramat added the WIP label Aug 11, 2018
@paramat paramat added this to the 5.0.0 milestone Aug 21, 2018
@paramat
Copy link
Contributor Author

paramat commented Sep 24, 2018

Updated to improve 'reset_box' API.

Now i need to consider your other suggestion.

@SmallJoker
Copy link
Member

Looks quite good already, although a structure like this would allow to assign a collisionbox to all available animations values, including custom ones:

player:set_properties({
	collisionbox = model.collisionbox[player_anim[name]] or model_collisionbox_default
})
-- Using a model definition like this:
{
	-- ... animations ...
	collisionbox = {
		lay = {-0.6, 0.0, -0.6, 0.6, 0.3, 0.6},
		sit = { ?? },
		-- whatever; custom animations?
	}
}

This would allow integrating it dynamically into set_animation and set_model. However, the old collisionbox definition would have to be converted to the new format (~3 lines in register_model)
Maybe I'm alone with this idea; feedback please.

@paramat
Copy link
Contributor Author

paramat commented Sep 24, 2018

It's quite a good idea but needs consideration.

@paramat paramat removed this from the 5.0.0 milestone Dec 20, 2018
@paramat paramat added this to the 5.1.0 milestone Jan 16, 2019
@paramat paramat removed this from the 5.1.0 milestone Mar 24, 2019
@paramat
Copy link
Contributor Author

paramat commented Jul 10, 2019

I've ground to a halt on this due to the consideration the suggested improvements need, which i don't feel like attending to.
The suggestions are good and i feel this PR is therefore a little messy.

@paramat
Copy link
Contributor Author

paramat commented Sep 22, 2020

My approach here was entirely wrong, and SmallJoker's suggestion is going in the right direction.
I am interested in this again, and have transferred the MT issue to MTG #2742

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

Successfully merging this pull request may close these issues.

None yet

2 participants