diff --git a/src/items/powerup.cpp b/src/items/powerup.cpp index c1a52a03b..bb71eca37 100644 --- a/src/items/powerup.cpp +++ b/src/items/powerup.cpp @@ -468,13 +468,44 @@ void Powerup::hitBonusBox(const ItemState &item_state) //seconds apart. If yes, then call getRandomPowerup again. If no, then break. for (int i = 0; i < 20; i++) { - // Determine the item based on index and time - random enough for - // the player, and reduces network synchronisation overhead. - // Dividing the time by 10 does not really allow exploiting the - // non-random selection (e.g. by displaying which item is collected - // next), since it's only around 83 ms - but it is bit more - // relaxed when client prediction should be a frame or so earlier. - int random_number = item_state.getItemId() + world->getTimeTicks() / 10; + // Determine a 'random' number based on time, index of the item, + // and position of the kart. The idea is that this process is + // randomly enough to get the right distribution of the powerups, + // does not involve additional network communication to keep + // client and server in sync, and is not exploitable: + // While it is not possible for a client to determine the item + // (the server will always finally determine which item a player + // receives), we need to make sure that people cannot modify the + // sources and display the item that will be collected next + // at a box - otherwise the player could chose the 'best' box. + // Using synchronised pseudo-random-generators would not prevent + // cheating, since the a cheater could determine the next random + // number that will be used. If we use the server to always + // send the information to the clients, we need to add a delay + // before items can be used. + // So instead we determine a random number that is based on: + // (1) The item id + // (2) The time + // (3) The position of the kart + // Using (1) means that not all boxes at a certain time for a kart + // will give the same box. Using (2) means that the item will + // change over time - even if the next item is displayed, it + // will mean a cheater has to wait, and because of the frequency + // of the time component it will also be difficult to get the + // item at the right time. Using (3) adds another cheat-prevention + // layer: even if a cheater is waiting for the right sequence + // of items, if he is overtaken the sequence will change. + // + // In order to increase the probability of correct client prediction + // in networking (where there might be 1 or 2 frames difference + // between client and server when collecting an item), the time + // is divided by 10, meaning even if there is one frame difference, + // the client will still have a 90% chance to correctly predict the + // item. We multiply the item with a 'large' (more or less random) + // number to spread the random values across the (typically 200) + // weights used in the PowerupManager - same for the position. + int random_number = item_state.getItemId()*31 + + world->getTimeTicks() / 10 + position*23; new_powerup = powerup_manager->getRandomPowerup(position, &n, random_number); if (new_powerup != PowerupManager::POWERUP_RUBBERBALL || diff --git a/src/items/powerup_manager.cpp b/src/items/powerup_manager.cpp index ea42fe58e..45430cc87 100644 --- a/src/items/powerup_manager.cpp +++ b/src/items/powerup_manager.cpp @@ -383,7 +383,8 @@ void PowerupManager::WeightsData::precomputeWeights() * The value returned matches the enum value of the random item if single. * In case of triple-item, the value will be the enum value plus * the number of existing powerups (= POWERUP_LAST-POWERUP_FIRST+1) - * \param rank The rank for which an item needs to be picked. + * \param rank The rank for which an item needs to be picked (between 0 + * and number_of_karts-1). * \param random_number A random number used to 'randomly' select the item * that was picked. */ @@ -392,10 +393,14 @@ int PowerupManager::WeightsData::getRandomItem(int rank, int random_number) // E.g. for battle mode with only one entry if(rank>(int)m_summed_weights_for_rank.size()) rank = m_summed_weights_for_rank.size()-1; - else if (rank<0) rank = 0; // E.g. battle mode + else if (rank<0) rank = 0; // E.g. battle mode, which has rank -1 const std::vector &summed_weights = m_summed_weights_for_rank[rank]; // The last entry is the sum of all previous entries, i.e. the maximum // value +#undef ITEM_DISTRIBUTION_DEBUG +#ifdef ITEM_DISTRIBUTION_DEBUG + int original_random_number = random_number; +#endif random_number = random_number % summed_weights.back(); // Put the random number in range [1;max of summed weights], // so for sum = N, there are N possible random numbers <= N. @@ -409,6 +414,12 @@ int PowerupManager::WeightsData::getRandomItem(int rank, int random_number) // We align with the beginning of the enum and return // We don't do more, because it would need to be decoded from enum later +#ifdef ITEM_DISTRIBUTION_DEBUG + Log::verbose("Powerup", "World %d rank %d random %d %d item %d", + World::getWorld()->getTimeTicks(), rank, random_number, + original_random_number, powerup); +#endif + return powerup + POWERUP_FIRST; } // WeightsData::getRandomItem @@ -553,8 +564,7 @@ void PowerupManager::computeWeightsForRace(int num_karts) * item for POSITION_BATTLE_MODE is returned. This function takes the weights * specified for all items into account by using a list which contains all * items depending on the weights defined. See updateWeightsForRace() - * \param pos Position of the kart (1<=pos<=number of karts) - ignored in - * case of a battle mode. + * \param pos Position of the kart (1<=pos<=number of karts). * \param n Number of times this item is given to the kart. * \param random_number A random number used to select the item. Important * for networking to be able to reproduce item selection. @@ -564,10 +574,6 @@ PowerupManager::PowerupType PowerupManager::getRandomPowerup(unsigned int pos, int random_number) { int powerup = m_current_item_weights.getRandomItem(pos-1, random_number); -#ifdef ITEM_DISTRIBUTION_DEBUG - Log::verbose("Powerup", "World %d pos %d random %d iten %d", - World::getWorld()->getTimeTicks(), pos, random_number, powerup); -#endif if(powerup > POWERUP_LAST) { powerup -= (POWERUP_LAST-POWERUP_FIRST+1);