Espyo / Pikifen

A fan-made Pikmin-based engine, built with flexibility in mind.
Other
57 stars 11 forks source link

Fixed `on_pikmin_land` event not triggering with Pikmin using the `impact` attack method or holding tools. #27

Closed Helodity closed 3 years ago

Helodity commented 3 years ago

The Problem

In order for on_pikmin_land to be triggered, the Pikmin needs to be touching a mob and have been in the air via a throw from a leader (mob::was_thrown) Whenever a pikmin lands on a mob, it runs pikmin_fsm::land_on_mob, which sets was_thrown to false, making it impossible for this Pikmin to trigger an on_pikmin_land event until it is thrown again. The problem appears to be caused by on_pikmin_land triggering when radii overlap, and pikmin_fsm::land_on_mob being triggered when hitboxes overlap. Given that the Pikmin hitboxes are larger than their radius, pikmin_fsm::land_on_mob is often triggered before on_pikmin_land is.

if(
    !h_ptr ||
    (
        pik_ptr->pik_type->attack_method == PIKMIN_ATTACK_LATCH &&
        !h_ptr->can_pikmin_latch
    )
) {
    //No good. Make it bounce back.
    m->speed.x *= -0.3;
    m->speed.y *= -0.3;
    return;
}
pik_ptr->stop_height_effect();
pik_ptr->focused_mob = mob_ptr;
pik_ptr->was_thrown = false;

Lines 2760 to lines 2775 of pikmin_fsm.cpp appear to be the cause of this bug. Dwarf Red Bulborbs (the mob that this bug was discovered on) can not be latched, latch method Pikmin would still be considered thrown. However, if you make the Dwarf Red Bulborb able to be latched onto, you will see that they are no longer crushed consistently when a Pikmin lands on top of them. This is because the statement would exit before was_thrown could be set to false. This latch check does not exist in pikmin_fsm::land_on_mob_while_holding, which is why pikmin holding tools can not crush a Dwarf Red Bulborb.

Proposed Solution

What these commits does is move the on_pikmin_land event detection and trigger into pikmin_fsm::land_on_mob. This ensures that, if the Pikmin was thrown, the event will be triggered before was_thrown was set to false. Also, Pikmin holding a tool are now able to trigger on_pikmin_land. Furthermore, this method reduces the amount of event checks that are done on average per frame, due to the event only being checked when a Pikmin lands on a mob. This should result in a performance benefit, although a performance change was unnoticeable while testing.

This method also includes a fairly large side effect. on_pikmin_land now uses hitboxes instead of the mob's radius. However, I believe this is a good thing, as hitboxes often follow a mob's shape more closely to a radius.