Backport improvements spawned from the first round of code review of kimden#45

Author: Nomagno

src/items/flyable.cpp

@@ -557,19 +557,10 @@ bool Flyable::hit(Kart *kart_hit, PhysicalObject* object)
     if (isOwnerImmunity(kart_hit))
         return false;
 
-    m_has_hit_something=true;
+     m_has_hit_something=true;
     if (kart_hit == NULL)
         return true;
 
-    ItemPolicy *policy = RaceManager::get()->getItemPolicy();
-    int leader_section_idx = policy->m_leader_section;
-    // If leader is not in a valid section, allow the hit
-    if (leader_section_idx <= -1)
-        return true;
-    // If blue flags are not enabled, ALSO allow the hit
-    if (!(policy->m_policy_sections[leader_section_idx].m_rules & ItemPolicyRules::IPT_BLUE_FLAGS))
-        return true;
-
     // Now, if blue flags are enabled and the leader is in a valid section, that's when we start to question if the hit could be forbidden (a lapper illegally hitting a lapping or vice versa).
     LinearWorld *lin_world = dynamic_cast<LinearWorld*>(World::getWorld());
     float track_length = Track::getCurrentTrack()->getTrackLength();
@@ -578,44 +569,11 @@ bool Flyable::hit(Kart *kart_hit, PhysicalObject* object)
 
     int sender_lap = lin_world->getFinishedLapsOfKart(m_owner->getWorldKartId());
     int recv_lap = lin_world->getFinishedLapsOfKart(kart_hit->getWorldKartId());
-
-    float minimum_distance_empirical = 200.0f;
-
-    float distance_normal = std::fabs(sender_distance - recv_distance);
-    float distance_complimentary = track_length - distance_normal;
-
-    bool across_finish_line;
-    bool forwards_throw;
-    if (distance_complimentary < distance_normal) {
-        across_finish_line = true;
-        if (sender_distance > recv_distance)
-            forwards_throw = true;
-        else
-            forwards_throw = false;
-    } else {
-        across_finish_line = false;
-    }
-
-    // if the distance is less than 5% from half the track length,
-    // it is nonsense to try to predict if the hit is across the finish line
-    if (distance_normal/track_length > 0.45 && distance_normal/track_length < 0.55)
-        across_finish_line = false;
-
-    // for too short tracks we instead take 1/5th of the track
-    if (track_length < 750.0f)
-        minimum_distance_empirical = track_length/5.0f;
-
-    bool hit_is_valid;
-    // sender with a 1 lap difference whose distance is less than an empirical number are almost certainly hitting each other across the start/finish line
-    if (across_finish_line && forwards_throw) {
-        hit_is_valid =  (recv_lap - sender_lap) == 1;
-    } else if (across_finish_line && !forwards_throw) {
-        hit_is_valid =  (sender_lap - recv_lap) == 1;
-    } else {
-        hit_is_valid = sender_lap == recv_lap;
-    }
-
-    return hit_is_valid;
+ 
+    // Blue flag settings could make the hit invalid, if it's between a lapper and a lapped or vice versa. Let Itempolicy decide this
+ 
+    ItemPolicy *policy = RaceManager::get()->getItemPolicy(); 
+    return policy->isHitValid(sender_distance, sender_lap, recv_distance, recv_lap, track_length);
 }   // hit
 
 // ----------------------------------------------------------------------------

src/items/item.cpp

@@ -125,7 +125,7 @@ void ItemState::initItem(ItemType type, const Vec3& xyz, const Vec3& normal, int
     setDisappearCounter();
 }   // initItem
 
-static int getRespawnTime(ItemState::ItemType type) {
+static int getRespawnTicks(ItemState::ItemType type) {
     auto& stk_config = STKConfig::get();
     switch (type)
     {
@@ -169,73 +169,7 @@ void ItemState::update(int ticks)
     }   // if collected
 
     ItemPolicy *policy = RaceManager::get()->getItemPolicy();
-    if (policy->m_policy_sections.size() == 0)
-        return;
-
-    int current_section = policy->m_leader_section;
-
-    if (current_section <= -1)
-        current_section = 0;
-
-    uint16_t rules_curr = policy->m_policy_sections[current_section].m_rules;
-    uint16_t rules_prev;
-    if (current_section > 0)
-        rules_prev = policy->m_policy_sections[current_section-1].m_rules;
-    else
-        rules_prev = rules_curr;
-
-    bool was_gum = (m_original_type==ITEM_BUBBLEGUM) || (m_type==ITEM_BUBBLEGUM_NOLOK) ||
-                  (m_original_type==ITEM_BUBBLEGUM_SMALL) || (m_type==ITEM_BUBBLEGUM_SMALL_NOLOK);
-
-    bool is_nitro = (m_type==ITEM_NITRO_SMALL) || (m_type==ITEM_NITRO_BIG);
-    bool was_nitro = (m_original_type==ITEM_NITRO_SMALL) || (m_original_type==ITEM_NITRO_BIG);
-
-    bool forbid_prev = ((rules_prev & ItemPolicyRules::IPT_FORBID_BONUSBOX) && m_type==ITEM_BONUS_BOX) ||
-                      ((rules_prev & ItemPolicyRules::IPT_FORBID_BANANA) && m_type==ITEM_BANANA)      ||
-                      ((rules_prev & ItemPolicyRules::IPT_FORBID_NITRO) && (is_nitro || was_nitro));
-
-    bool forbid_curr = ((rules_curr & ItemPolicyRules::IPT_FORBID_BONUSBOX) && m_type==ITEM_BONUS_BOX) ||
-                      ((rules_curr & ItemPolicyRules::IPT_FORBID_BANANA) && m_type==ITEM_BANANA)      ||
-                      ((rules_curr & ItemPolicyRules::IPT_FORBID_NITRO) && (is_nitro || was_nitro));
-
-    int fuel_mode = std::get<0>(RaceManager::get()->getFuelAndQueueInfo());
-    if (fuel_mode == 0 && m_type == ITEM_TYRE_CHANGE && m_compound == 123) // If we're a fuel tyre changer and fuel is off, don't spawn
-    	forbid_curr = true;
-
-
-    // Gums that were switched into nitro are NEVER forbidden
-    bool instant = false;
-    if (was_gum && is_nitro) {
-        instant = true;
-    } else {
-        instant = false;
-    }
-
-    auto& stk_config = STKConfig::get();
-    // There's redundant cases here, but it is like this for maintainability
-    if (forbid_prev && forbid_curr)
-        m_ticks_till_return = stk_config->time2Ticks(99999);
-    else if (!forbid_prev && forbid_curr)
-        m_ticks_till_return = stk_config->time2Ticks(99999);
-    else if (forbid_prev && !forbid_curr) {
-        int respawn_ticks = getRespawnTime(m_type);
-        // If the ticks till return are abnormally high, set them back to normal.
-        // If we don't do it like this, it will set the ticks till return perpetually
-        // when transitioning from a section without to a section with this item type allowed.
-        if (m_ticks_till_return > 10*respawn_ticks)
-            m_ticks_till_return = respawn_ticks;
-    }
-    else if (!forbid_prev && !forbid_curr) {
-        // Nothing to do
-        // This wouldn't be needed normally, but we do it in case of switched items
-        int respawn_ticks = getRespawnTime(m_type);
-        if (m_ticks_till_return > 10*respawn_ticks && m_type != ITEM_EASTER_EGG)
-            m_ticks_till_return = respawn_ticks;
-    }
-    if (instant) {
-        m_ticks_till_return = 0;
-    }
-
+    m_ticks_till_return = policy->computeItemTicksTillReturn(m_original_type, m_type, getRespawnTicks(m_type), m_ticks_till_return);
 }   // update
 
 // ----------------------------------------------------------------------------
@@ -263,7 +197,7 @@ void ItemState::collected(const Kart *kart)
     }
     else
     {
-        m_ticks_till_return = getRespawnTime(m_type);
+        m_ticks_till_return = getRespawnTicks(m_type);
     }
 
     if (RaceManager::get()->isBattleMode())

src/items/item.hpp

@@ -24,7 +24,7 @@
  *  Defines the various collectibles and weapons of STK.
  */
 
-#include "karts/kart.hpp"
+//#include "karts/kart.hpp"
 #include "utils/cpp2011.hpp"
 #include "utils/leak_check.hpp"
 #include "utils/log.hpp"

src/items/powerup_manager.cpp

@@ -95,42 +95,37 @@ void PowerupManager::unloadPowerups()
     }
 }   // removeTextures
 
+// Must match the order of PowerupType in powerup_manager.hpp!!
+static const std::string powerup_names[] = {
+    "nothing",            /* Nothing */
+    "bubblegum", "cake", "bowling", "zipper", "plunger", "switch",
+    "swatter", "rubber-ball", "parachute", "nitro-hack", "electro-shield",
+    "mini-wish", "anchor"
+};
+
 //-----------------------------------------------------------------------------
 /** Determines the powerup type for a given name.
  *  \param name Name of the powerup to look up.
  *  \return The type, or POWERUP_NOTHING if the name is not found
  */
-PowerupManager::PowerupType
-    PowerupManager::getPowerupType(const std::string &name) {
-    // Must match the order of PowerupType in powerup_manager.hpp!!
-    static const std::string powerup_names[] = {
-        "nothing",            /* Nothing */
-        "bubblegum", "cake", "bowling", "zipper", "plunger", "switch",
-        "swatter", "rubber-ball", "parachute", "nitro-hack", "electro-shield",
-        "mini-wish", "anchor"
-    };
-
+PowerupManager::PowerupType PowerupManager::getPowerupType(const std::string &name)
+{
     for(unsigned int i=POWERUP_FIRST; i<=POWERUP_LAST; i++)
     {
-        if(name=="") return POWERUP_NOTHING;
-        if(powerup_names[i]==name) return(PowerupType)i;
+        if (name=="")
+            return POWERUP_NOTHING;
+        if (powerup_names[i]==name)
+            return(PowerupType)i;
     }
     return POWERUP_NOTHING;
 }   // getPowerupType
 
-std::string
-    PowerupManager::getPowerupAsString(PowerupManager::PowerupType type) {
-    // Must match the order of PowerupType in powerup_manager.hpp!!
-    static const std::string powerup_names[] = {
-        "nothing",            /* Nothing */
-        "bubblegum", "cake", "bowling", "zipper", "plunger", "switch",
-        "swatter", "rubber-ball", "parachute", "nitro-hack", "electro-shield",
-        "mini-wish", "anchor"
-    };
+std::string PowerupManager::getPowerupAsString(PowerupManager::PowerupType type)
+{
     int size = sizeof(powerup_names)/sizeof(*powerup_names);
     if (type == POWERUP_NOTHING) return "nothing";
     if (size < type-POWERUP_FIRST) return "nothing";
-    return powerup_names[type-POWERUP_FIRST+1];
+    return powerup_names[type - POWERUP_FIRST + 1];
 }   // getPowerupAsString
 
 

src/karts/kart.cpp

@@ -528,8 +528,8 @@ void Kart::reset()
 
     unsetSquash();
 
-    item_amount_last_lap = 0;
-    item_type_last_lap = PowerupManager::POWERUP_NOTHING;
+    m_item_amount_last_lap = 0;
+    m_item_type_last_lap = PowerupManager::POWERUP_NOTHING;
 
     m_last_used_powerup    = PowerupManager::POWERUP_NOTHING;
     m_race_position        = m_initial_position;
@@ -678,9 +678,9 @@ void Kart::instantSpeedIncrease(unsigned int category, float add_max_speed,
 
 // -----------------------------------------------------------------------------
 void Kart::setSlowdown(unsigned int category, float max_speed_fraction,
-                       int fade_in_time)
+                       int fade_in_time, int duration)
 {
-    m_max_speed->setSlowdown(category, max_speed_fraction,  fade_in_time);
+    m_max_speed->setSlowdown(category, max_speed_fraction,  fade_in_time, duration);
 }   // setSlowdown
 
 // -----------------------------------------------------------------------------
@@ -3203,36 +3203,10 @@ void Kart::updatePhysics(int ticks)
     // Cap speed if necessary
     const Material *m = getMaterial();
 
-    ItemPolicy *itempolicy = RaceManager::get()->getItemPolicy();
-    bool is_restart = itempolicy->m_virtualpace_code <= -3;
-    bool did_restart = false;
-    if (is_restart) {
-        // Reaffirm the penalty in case someone tried to be funny and pit for tyres in the middle of a safety car restart
-        m_max_speed->setSlowdown(MaxSpeed::MS_DECREASE_STOP, 0.1f, stk_config->time2Ticks(0.1f), -1);
-        int restart_time = -(itempolicy->m_virtualpace_code + 3);
-        float gap = itempolicy->m_policy_sections[itempolicy->m_leader_section].m_virtualpace_gaps;
-        gap *= getPosition();
-        restart_time += gap;
-        int current_time = World::getWorld()->getTime();
-        if (current_time > restart_time) {
-            // Set slowdown time to 0 (disable it) if its time to restart
-            m_max_speed->setSlowdown(MaxSpeed::MS_DECREASE_STOP, 0.1f, stk_config->time2Ticks(0.1f), stk_config->time2Ticks(0));
-            did_restart = true;
-        }
-    }
-
-    bool is_last = getPosition() == RaceManager::get()->getNumberOfKarts();
 
-    if (is_last && did_restart) {
-        itempolicy->m_virtualpace_code = -1;
-        itempolicy->m_restart_count = -1;
-    }
-
-    // the only reason such a ridiculous infinite pit penalty (-1) can be given is if it's a virtual pace car restart
-    // plainly, the only reason this exists is because first place won't get its penalty overturned if for some reason
-    if (itempolicy->m_virtualpace_code == -1 && m_max_speed->getSpeedDecreaseTicksLeft(MaxSpeed::MS_DECREASE_STOP) == -1) {
-        m_max_speed->setSlowdown(MaxSpeed::MS_DECREASE_STOP, 0.1f, stk_config->time2Ticks(0.1f), stk_config->time2Ticks(0));
-    }
+    ItemPolicy *item_policy = RaceManager::get()->getItemPolicy();
+    item_policy->enforceVirtualPaceCarRulesForKart(this);
+ 
 
     float min_speed =  m && m->isZipper() ? m->getZipperMinSpeed() : -1.0f;
     m_max_speed->setMinSpeed(min_speed);

src/karts/kart.hpp

@@ -422,8 +422,8 @@ class Kart : public Moveable
     void setStints (std::vector<std::tuple<unsigned, unsigned>> x) { m_stints = x; }  
 
     // amount in previous lap
-    int item_amount_last_lap;
-    PowerupManager::PowerupType item_type_last_lap;
+    int m_item_amount_last_lap;
+    PowerupManager::PowerupType m_item_type_last_lap;
 
     bool m_is_refueling;
     float m_target_refuel;
@@ -633,7 +633,7 @@ class Kart : public Moveable
      *  \param max_speed_fraction Fraction of top speed to allow only.
      *  \param fade_in_time How long till maximum speed is capped. */
     virtual void   setSlowdown(unsigned int category, float max_speed_fraction,
-                               int fade_in_time);
+                               int fade_in_time, int duration = -1);
     // ----------------------------------------------------------------------------------------
     /** Returns how much increased speed time is left over in the given
      *  category.

src/modes/linear_world.cpp

@@ -43,7 +43,7 @@
 #include "network/stk_host.hpp"
 #include "network/stk_peer.hpp"
 #include "race/race_manager.hpp"
-#include "race/itempolicy.hpp"
+#include "race/item_policy.hpp"
 #include "race/history.hpp"
 #include "states_screens/race_gui_base.hpp"
 #include "tracks/check_manager.hpp"
@@ -487,46 +487,17 @@ void LinearWorld::newLap(unsigned int kart_index)
     // duplicated race positions as well.
     updateRacePosition();
 
-    ItemPolicy *itempolicy = RaceManager::get()->getItemPolicy();
-
-    int sec = itempolicy->applyRules(kart, kart_info.m_finished_laps, World::getWorld()->getTime());
-    if (kart->getPosition() == 1) {
-        itempolicy->m_leader_section = sec;
-        int start_lap = itempolicy->m_policy_sections[sec].m_section_start;
-        int16_t rules = itempolicy->m_policy_sections[sec].m_rules;
-        bool do_virtualpace = rules & ItemPolicyRules::IPT_VIRTUALPACE;
-        bool do_unlapping = rules & ItemPolicyRules::IPT_UNLAPPING;
-        if (do_virtualpace && start_lap == kart_info.m_finished_laps) {
-            itempolicy->m_restart_count = 0;
-            itempolicy->m_virtualpace_code = do_unlapping
-                                             ? start_lap // Lappings must slow down when they reach the lead lap
-                                             : -2;       // Lappings must slow down as soon as possible
-        }
-    }
-    kart->m_tyres->m_deg_mult = itempolicy->m_policy_sections[sec].m_deg_mult;
-    kart->m_tyres->m_c_fuel_rate = itempolicy->m_policy_sections[sec].m_fuel_mult;
-
-    kart->item_type_last_lap = kart->getPowerup()->getType();
-    kart->item_amount_last_lap = kart->getPowerup()->getNum();
-
-    bool slowed_down = false;
-    if (itempolicy->m_virtualpace_code == kart_info.m_finished_laps || itempolicy->m_virtualpace_code == -2) {
-        auto& stk_config = STKConfig::get();
-        itempolicy->m_restart_count += 1;
-        kart->m_max_speed->setSlowdown(MaxSpeed::MS_DECREASE_STOP, 0.1f, stk_config->time2Ticks(0.1f), stk_config->time2Ticks(99999));
-        slowed_down = true;
-    }
+    ItemPolicy *item_policy = RaceManager::get()->getItemPolicy();
+
+    int sec = item_policy->applyRules(kart, kart_info.m_finished_laps, World::getWorld()->getTime(), RaceManager::get()->getNumLaps());
+
+    kart->m_item_type_last_lap = kart->getPowerup()->getType();
+    kart->m_item_amount_last_lap = kart->getPowerup()->getNum();
+    kart->m_tyres->m_deg_mult = item_policy->m_policy_sections[sec].m_deg_mult;
+    kart->m_tyres->m_c_fuel_rate = item_policy->m_policy_sections[sec].m_fuel_mult;
 
-    bool is_last = itempolicy->m_restart_count == RaceManager::get()->getNumberOfKarts();
-    // Technically there could be ghosts but you can't get into that situation. Probably.
-    // If the kart is last, also fire the virtual pace car restart procedure
-    if (slowed_down && is_last) {
-        int time = World::getWorld()->getTime();
-        time = (-time) - 3;
-        // code till be added to 3 and inverted to get time
-        itempolicy->m_virtualpace_code = time;
-    }
 
+    item_policy->checkAndApplyVirtualPaceCarRules(kart, sec, kart_info.m_finished_laps);
 
     // Race finished
     // We compute the exact moment the kart crossed the line

src/network/packet_types.hpp

@@ -38,7 +38,7 @@ enum LobbyEvent: uint8_t
     LE_CONNECTION_REQUESTED = 1, // a connection to the server
     LE_CONNECTION_REFUSED, // Connection to server refused
     LE_CONNECTION_ACCEPTED, // Connection to server accepted
-    LE_ITEMPOLICY, // inform client about item policy
+    LE_ITEM_POLICY, // inform client about item policy
     LE_SERVER_INFO, // inform client about server info
     LE_REQUEST_BEGIN, // begin of kart selection
     LE_UPDATE_PLAYER_LIST, // inform client about player list update

src/network/protocols/client_lobby.cpp

@@ -208,7 +208,7 @@ bool ClientLobby::notifyEvent(Event* event)
         case LE_UPDATE_PLAYER_LIST:    updatePlayerList(event);    break;
         case LE_CHAT:                  handleChat(event);          break;
         case LE_CONNECTION_ACCEPTED:   connectionAccepted(event);  break;
-        case LE_ITEMPOLICY:            handleItemPolicy(event);    break;
+        case LE_ITEM_POLICY:            handleItemPolicy(event);    break;
         case LE_SERVER_INFO:           handleServerInfo(event);    break;
         case LE_PLAYER_DISCONNECTED :  disconnectedPlayer(event);  break;
         case LE_CONNECTION_REFUSED:    connectionRefused(event);   break;