minetest / minetest_game

Minetest Game - A lightweight and well-maintained base for modding [https://github.com/minetest/minetest/]
http://minetest.net/
Other
1.43k stars 578 forks source link

The /home command does not work when mounted on a boat #2722

Open FeXoR-o-Illuria opened 4 years ago

FeXoR-o-Illuria commented 4 years ago

When mounted on a boat /home won't do anything.

Reproduce: 1) Type /sethome (With home priv enabled) 2) Place and mount a boat (somewhere clearly not at your home position) 3) Type /home and notice that nothing happens

(Some other means of teleportation like /spawn on the servers I tried are affected. Some other mountables like horses may also be affected but I did not check that)

sfan5 commented 4 years ago

related minetest/minetest#9823

FeXoR-o-Illuria commented 4 years ago

Thanks for your quick reaction @sfan5 ! ;) Sorry for reporting a bug already discussed and closed. (I only searched for open bugs) Then again I don't see why it was closed.

I don't see a need for detaching the player in the first place. But the teleport should happen! (If it's decided that detaching is the better choice I'm fine with that, too)

paramat commented 4 years ago

That issue was closed because:

Closing this as attached players should not be able to teleport to escape attachment, and the issue author has not made any other request. We could possibly reopen this if the author has another request.

However, this issue is a different. It would be allowable, if desired, to detach the player from a boat when using /sethome, as the boat is not an entity a server uses to confine a player. This is not a bug as i think it is reasonable for a player to be expected to dismount a vehicle before teleporting, because it makes no sense for the vehicle to teleport with you. So i also think this is low priority.

FeXoR-o-Illuria commented 4 years ago

What a bout an additional bool values for a vehicle to have like allowTpWhenAttached and detatchBeforeTp ?

And then handle in the teleportation code: If allowTp is false only send a message to player initializing TP and player that was to be TPed like "TP not allowed while attached to [vehicle name]". If allowTp is true and detatchBeforeTp is false TP with vehicle. If both true detach the player and then TP him.

That way different uses of vehicles would be supported, For the plain MTG I do not see any problem to just teleport with the boat/mine cart.

Wuzzy2 commented 4 years ago

Just do a player:set_detach() before the teleport. So you teleport, but you leave your boat behind. You could limit this auto-detach feature to boats, carts and other “official” MTG vehicles to avoid screwing up external mods. If player was attached to another object, print an error message (like for /teleport).

Wuzzy2 commented 4 years ago

I like FeXoR-o-Illuria's idea a lot. Ideally, this should be a change in Minetest, and be more generally worded (use setPos instead of teleport; the check should apply to set_pos to make it more generic).

paramat commented 4 years ago

What a bout an additional bool values for a vehicle to have like allowTpWhenAttached and detatchBeforeTp ? And then handle in the teleportation code:

I disagree this is worth the complexity, especially since it affects engine code. It is fairly obvious a player should detach before teleporting, or if not obvious, when nothing happens a player will soon realise they should detach from a vehicle. A vehicle is something that locks you to a position and overrides your freedom of movement, so not teleporting makes sense.

How about we simply make the 'sethome' mod display a warning message if attached when using /sethome? The player is then informed why they cannot teleport. Essentially, the same approach as what we recently did in the engine in https://github.com/minetest/minetest/pull/9824 I would support this.