Make mobs ride minecarts #3507

Merged
ancientmarinerdev merged 5 commits from mobs_in_minecarts into master 2023-03-20 14:42:40 +01:00
1 changed files with 30 additions and 0 deletions

View File

@ -66,6 +66,7 @@ local function register_entity(entity_id, mesh, textures, drop, on_rightclick, o
on_rightclick = on_rightclick,
_driver = nil, -- player who sits in and controls the minecart (only for minecart!)
_passenger = nil, -- for mobs
_punched = false, -- used to re-send _velocity and position
_velocity = {x=0, y=0, z=0}, -- only used on punch
_start_pos = nil, -- Used to calculate distance for “On A Rail” achievement
@ -86,6 +87,7 @@ local function register_entity(entity_id, mesh, textures, drop, on_rightclick, o
local data = minetest.deserialize(staticdata)
if type(data) == "table" then
self._railtype = data._railtype
self._passenger = data._passenger
end
self.object:set_armor_groups({immortal=1})
@ -177,6 +179,8 @@ local function register_entity(entity_id, mesh, textures, drop, on_rightclick, o
cart.on_activate_by_rail = on_activate_by_rail
local passenger_attach_position = vector.new(0, -1.75, 0)
function cart:on_step(dtime)
local ctrl, player = nil, nil
if self._driver then
@ -212,6 +216,32 @@ local function register_entity(entity_id, mesh, textures, drop, on_rightclick, o
end
end

I think we're going to need a timer on this. If you set a timer to 0 on the object, and then add dtime to it, after timer > x, then run this code and reset it 0.

The reason for this, is on_step runs at least 20 times a second. Most of the performance profiling I do, the 2 most expensive operations the usually flag up on the report are usually looking for a node near, or looking for an object near. If you're doing that 20 times a second, even for 1 block, it can be really expensive. If you have many minecarts, even worse.

It should run that code the minimum amount to function, so will require some testing. x could be 0.5 (so twice a second, which would be 10 times less costly. I would also consider every 1 second, or ever 0.75 for example, just see what works, and what doesn't at fastest minecart setting. This is trial and error or course. We need the most infrequent that still functions.

I think we're going to need a timer on this. If you set a timer to 0 on the object, and then add dtime to it, after timer > x, then run this code and reset it 0. The reason for this, is on_step runs at least 20 times a second. Most of the performance profiling I do, the 2 most expensive operations the usually flag up on the report are usually looking for a node near, or looking for an object near. If you're doing that 20 times a second, even for 1 block, it can be really expensive. If you have many minecarts, even worse. It should run that code the minimum amount to function, so will require some testing. x could be 0.5 (so twice a second, which would be 10 times less costly. I would also consider every 1 second, or ever 0.75 for example, just see what works, and what doesn't at fastest minecart setting. This is trial and error or course. We need the most infrequent that still functions.
Review

Tried to add a timer, but didn't worked out. At least I didn't managed to make it work... But, nonetheless I used the math.random(1,20) > 15 strategy. I hope this makes performance better.

Tried to add a timer, but didn't worked out. At least I didn't managed to make it work... But, nonetheless I used the math.random(1,20) > 15 strategy. I hope this makes performance better.
-- Grab mob
if math.random(1,20) > 15 and not self._passenger then
if self.name == "mcl_minecarts:minecart" then
local mobsnear = minetest.get_objects_inside_radius(self.object:get_pos(), 1.3)
for n=1, #mobsnear do
local mob = mobsnear[n]
if mob then
local entity = mob:get_luaentity()
if entity and entity.is_mob then
self._passenger = entity
mob:set_attach(self.object, "", passenger_attach_position, vector.zero())
break
end
end
end
end

I'm wondering if rather than running this code 20 times a second, we call a function from the death handling.

If you look in physics.lua in mcl_mobs for:

function mob_class:check_for_death(cause, cmi_cause)

It calls:

local function death_handle(self)

In there feels the most appropriate place to check for the minecart and detach. That way the code and looking for the minecart happens once only on death, and not multiple times a second. I would have the minecart logic in a function in minecarts, and the death_handle calls calls that function with mob as parameter. This function would check for a minecart nearby, hopefully find the most appropriate and update it.

I don't want to discourage you as your problem solving and code is good, will likely work and maybe before profiling, I might have done a similar thing. I do think, however that looking for the most performant solution will really help this game scale for servers and help things run good and lag free :).

I'm wondering if rather than running this code 20 times a second, we call a function from the death handling. If you look in physics.lua in mcl_mobs for: function mob_class:check_for_death(cause, cmi_cause) It calls: local function death_handle(self) In there feels the most appropriate place to check for the minecart and detach. That way the code and looking for the minecart happens once only on death, and not multiple times a second. I would have the minecart logic in a function in minecarts, and the death_handle calls calls that function with mob as parameter. This function would check for a minecart nearby, hopefully find the most appropriate and update it. I don't want to discourage you as your problem solving and code is good, will likely work and maybe before profiling, I might have done a similar thing. I do think, however that looking for the most performant solution will really help this game scale for servers and help things run good and lag free :).
Review

Or you can set self._passenger to the ObjectRef/luaentity and then just do an ObjectRef validity test on self._passenger. Setting self._passenger to the name of the mob doesn't seem to useful.

Also you need to consider that the radius of 1.3 will pick up some entities that aren't the riding mob. If you know the exact position of an entity you can use a radius of 0 to get the entity. But it is moot in this case.

Or you can set self._passenger to the ObjectRef/luaentity and then just do an ObjectRef validity test on self._passenger. Setting self._passenger to the name of the mob doesn't seem to useful. Also you need to consider that the radius of 1.3 will pick up some entities that aren't the riding mob. If you know the exact position of an entity you can use a radius of 0 to get the entity. But it is moot in this case.
Review

Changed the self._passenger for the entity instead of the entity.name. By this I could run the check_for_death. A much better approach, thanks for the suggestions!

Changed the self.\_passenger for the entity instead of the entity.name. By this I could run the check_for_death. A much better approach, thanks for the suggestions!

Hi. I probably didn't communicate well what I mean. I meant that this code is called 20 times a second. By putting the logic in the death_handle function, it will only be called on death. So the death_handle function will call a function that checks if a minecart was near that it was in, and remove passenger if it was in it. That way, it is only called once, and that is when the mob dies.

Hi. I probably didn't communicate well what I mean. I meant that this code is called 20 times a second. By putting the logic in the death_handle function, it will only be called on death. So the death_handle function will call a function that checks if a minecart was near that it was in, and remove passenger if it was in it. That way, it is only called once, and that is when the mob dies.
Review

Oh now I got it! As I mentioned before, probably the best way to handle mobs in minecarts/boats would be to have a dedicated function in mcl_mobs, but since the boat riding code was on the boat init.lua I though on adding this on the minecart as well.

I personally don't know if it's a good thing to search for an attached minecart everytime a mob dies, as probably there will be more mobs dying than mobs riding minecarts.

Oh now I got it! As I mentioned before, probably the best way to handle mobs in minecarts/boats would be to have a dedicated function in mcl_mobs, but since the boat riding code was on the boat init.lua I though on adding this on the minecart as well. I personally don't know if it's a good thing to search for an attached minecart everytime a mob dies, as probably there will be more mobs dying than mobs riding minecarts.

I think you're right in having minecart logic in minecart.

We do have a dependency and it doesn't seem to be documented at present. I am not sure if minecarts depend on mobs or mobs depend on minecart and this will need some thinking. This does work however.

I did make the death check less frequent. It doesn't need to be checked 20 times a second, so I have made it every second.

Another issue I spotted, was if you have a passenger, and you shut down, they vanish. I have now ensured on load it does load the mob.

I think you're right in having minecart logic in minecart. We do have a dependency and it doesn't seem to be documented at present. I am not sure if minecarts depend on mobs or mobs depend on minecart and this will need some thinking. This does work however. I did make the death check less frequent. It doesn't need to be checked 20 times a second, so I have made it every second. Another issue I spotted, was if you have a passenger, and you shut down, they vanish. I have now ensured on load it does load the mob.
-- Make room in the minecart after the mob dies
elseif self._passenger then
if math.random(1,20) == 1 then
local dead = self._passenger:check_for_death()
if dead == true then
self._passenger = nil
end
end
end
-- Drop minecart if it isn't on a rail anymore
if self._last_float_check >= mcl_minecarts.check_float_time then
pos = self.object:get_pos()