agoragames / leaderboard

Leaderboards backed by Redis in Ruby
https://rubygems.org/gems/leaderboard
MIT License
478 stars 65 forks source link

Update score only when the new score is better #31

Closed guardianblue closed 11 years ago

guardianblue commented 11 years ago

Currently, rank_member method overwrites existing score. However, sometimes we want to update the score only if the new one is better.

Below is the suggested code for this feature.

  # Rank a member in the leaderboard, if the new score is better than the existing one
  #
  # @param member [String] Member name.
  # @param score [float] Member score.
  # @param member_data [Hash] Optional member data.
  def rank_member_if_better(member, score, member_data = nil)
    rank_member_in_if_better(@leaderboard_name, member, score, member_data)
  end

  # Rank a member in the named leaderboard, if the new score is better than the existing one
  #
  # @param leaderboard_name [String] Name of the leaderboard.
  # @param member [String] Member name.
  # @param score [float] Member score.
  # @param member_data [Hash] Optional member data.
  def rank_member_in_if_better(leaderboard_name, member, score, member_data)

    @redis_connection.watch(member) do
      old_score = @redis_connection.zscore(leaderboard_name, member)
      better = !old_score or (@reverse ? score < old_score : score > old_score)
      if better
        @redis_connection.multi do |transaction|
          transaction.zadd(leaderboard_name, score, member)
          if member_data
            transaction.hmset(member_data_key(leaderboard_name, member), *member_data.to_a.flatten)
          end
        end
      end
    end
  end
czarneckid commented 11 years ago

Interesting idea. I've implemented a little more generic way of doing conditional logic to dictate whether or not a member should be ranked in the leaderboard. The lambda would be passed the member, current score, score and the member data.

You can find this code in the version-3-proposal-rank-member-if branch. From the README in that proposal:

highscore_check = lambda do |member, current_score, score, member_data|
  return true if current_score.nil?
  return true if score > current_score
  false
end

highscore_lb.rank_member_if(highscore_check, 'david', 1337)
highscore_lb.score_for('david')
 => 1337.0
highscore_lb.rank_member_if(highscore_check, 'david', 1336)
highscore_lb.score_for('david')
 => 1337.0
highscore_lb.rank_member_if(highscore_check, 'david', 1338)
highscore_lb.score_for('david')
 => 1338.0
guardianblue commented 11 years ago

Yes, allow user to pass a block is much more generic. Nice!

guardianblue commented 11 years ago

By the way, I think I have made a mistake with the determination of "better"

Correct code should be:

better = (!old_score) || (@reverse ? (score < old_score) : (score > old_score))
czarneckid commented 11 years ago

Updated the code to add a final parameter passed to the rank_conditional with the leaderboard options.

czarneckid commented 11 years ago

Code from the version-3-proposal-rank-member-if branch has been merged into the version-3 branch. I'll probably release 3.0.0.rc3 with this functionality and then finally release 3.0.0, but you can start using this functionality now from the version-3 branch.