Make golem go home. Fixes #3288 #3929

Codiac · 2023-09-10T13:45:05+02:00

Codiac commented

2023-09-10 13:45:05 +02:00

Golem never goes home, now it does.

This fixes the incorrect calling of go_path, and makes it teleport if it is far away.

It does not address the issues in mobs navigating, so in complex navigational areas it will get stuck.

Fixes #3288

Testing

In general you can watch a golem and when it gets a long way from home it will walk or teleport home.

If you add follow = { "mcl_hamburger:hamburger" }, you can lead it away and when you are at the distance you want to test swap items and wait 10 seconds.

Golem never goes home, now it does. This fixes the incorrect calling of go_path, and makes it teleport if it is far away. It does not address the issues in mobs navigating, so in complex navigational areas it will get stuck. Fixes #3288 ### Testing In general you can watch a golem and when it gets a long way from home it will walk or teleport home. If you add ```follow = { "mcl_hamburger:hamburger" },``` you can lead it away and when you are at the distance you want to test swap items and wait 10 seconds.

kneekoo commented

2023-09-11 01:14:28 +02:00

I'm not a fan of the teleportation solution. If it can't pathfind back, so be it.

Imagine an unsuspecting player trying to kill a golem (with Knockback or Punch), which pushes it out of tele_dist, and you see it just vanish. They would think something went wrong because there are no drops.

Also, when we'll get our water mechanics in order, we'll be able to move mobs around with water pathways. If we have this teleporting band-aid for mobs, long-distance transportation won't work.

I'm not a fan of the teleportation solution. If it can't pathfind back, so be it. Imagine an unsuspecting player trying to kill a golem (with Knockback or Punch), which pushes it out of `tele_dist`, and you see it just vanish. They would think something went wrong because there are no drops. Also, when we'll get our water mechanics in order, we'll be able to move mobs around with water pathways. If we have this teleporting band-aid for mobs, long-distance transportation won't work.

Codiac commented

2023-09-11 03:29:44 +02:00

HI @kneekoo, I think if the golem is attacking then it shouldn't teleport or walk home, so I'll put in a check for that and skip the whole thing if it's attacking. Maybe there are other states that should also skip this?

I think the navigation is very broken, has been very broken for a long time, and has no one actively working on it, therefore I think putting workarounds in place for that is fair enough.

What about adding a setting to enable it, would that be OK?

Something like minetest.settings:get_bool("mcl_allow_golem_teleport",false) or maybe a more generic mcl_allow_mob_nav_hacks that would make it easy to track down and remove when the nav is fixed?

HI @kneekoo, I think if the golem is attacking then it shouldn't teleport or walk home, so I'll put in a check for that and skip the whole thing if it's attacking. Maybe there are other states that should also skip this? I think the navigation is very broken, has been very broken for a long time, and has no one actively working on it, therefore I think putting workarounds in place for that is fair enough. What about adding a setting to enable it, would that be OK? Something like ```minetest.settings:get_bool("mcl_allow_golem_teleport",false)``` or maybe a more generic ```mcl_allow_mob_nav_hacks``` that would make it easy to track down and remove when the nav is fixed?

SmokeyDope commented

2023-09-11 03:38:05 +02:00

I personally think that is an exellent stop gap solution Codiac and like the idea of it being a setting. Teleportation is not an ideal solution but its better than nothing until a proper fix to pathfinding is implemented. Just my 2c

kneekoo commented

2023-09-11 04:03:02 +02:00

Temporary workarounds are the enemy of good solutions. The good thing is that in this particular case it's not a tragedy if an iron golem gets lost, because the villagers summon new ones if they when they don't have golems. So we can definitely put up with the suboptimal pathfinding here.

Codiac commented

2023-09-11 04:50:18 +02:00

I like it, you don't, I think my proposal is a reasonable compromise.

kneekoo commented

2023-09-11 06:32:26 +02:00

This is not a matter of likes. It's about not creating unexpected and unwanted behaviour that affects the gameplay. And it's all for a "renewable" mob.

ancientmarinerdev commented

2023-09-11 15:53:21 +02:00

Now we have mobs jumping better, if outside distance, maybe we just make them mobs turn and walk in that direction towards home. If you make it do that like 70% of the time, it'll probably get there eventually. Instant teleport isn't a great experience.

If you look at check_herd logic, they do something similar.

Now we have mobs jumping better, if outside distance, maybe we just make them mobs turn and walk in that direction towards home. If you make it do that like 70% of the time, it'll probably get there eventually. Instant teleport isn't a great experience. If you look at check_herd logic, they do something similar.

Codiac commented

2023-09-12 08:59:01 +02:00

The point of me offering up patches I am carrying is so I don't have to carry the patch. If I still need to do so after the PR then there isn't a point in me doing it.

I'm happy to add stuff to fit other peoples likes and use-cases, but I'm not willing to drop the bits that mean I will still need to carry a patch. In that circumstance someone else will need to take over the PR.

Pitch and yaw is already handled by the movement code, it looks fine to me in testing.

The point of me offering up patches I am carrying is so I don't have to carry the patch. If I still need to do so after the PR then there isn't a point in me doing it. I'm happy to add stuff to fit other peoples likes and use-cases, but I'm not willing to drop the bits that mean I will still need to carry a patch. In that circumstance someone else will need to take over the PR. Pitch and yaw is already handled by the movement code, it looks fine to me in testing.

Codiac commented

2023-09-26 02:46:42 +02:00

@ancientmarinerdev There is now a setting in the Experimental section to control the golem teleport. I think this is a reasonable compromise until the navigation is not appalling. Can we merge this PR now?

the-real-herowl added the

mobs

Testing / Retest

labels 2023-10-07 10:22:03 +02:00

ancientmarinerdev commented

2023-10-15 19:22:04 +02:00

I personally have no objections with this going in with the changes added in (check for not attacking, and the settings switch). I have not tested this though. Code seems logical.

I did wonder about setting in the follow state afterwards, but maybe that is right for the metalheads.

I personally have no objections with this going in with the changes added in (check for not attacking, and the settings switch). I have not tested this though. Code seems logical. I did wonder about setting in the follow state afterwards, but maybe that is right for the metalheads.

the-real-herowl force-pushed golem_nav from 74f5f52f95 to da79137105

2023-10-15 22:44:27 +02:00

Compare

the-real-herowl force-pushed golem_nav from da79137105 to 1850c5ca7e

2023-11-06 22:03:19 +01:00

Compare

the-real-herowl force-pushed golem_nav from 1850c5ca7e to 71282e196e

2023-11-06 22:46:31 +01:00

Compare

the-real-herowl approved these changes 2023-11-06 23:29:43 +01:00

the-real-herowl merged commit accb8742dd into master

2023-11-06 23:31:47 +01:00