Help Siege Reward for aCis

[Williams]

Hello i'm creating this code for the clan that take the castle.

 

created rewardClanWinner () method in Player.java , sure he rewards the winning clan.

 

method:

    public Clan rewardClanWinner()
    {
        for (Player player : getClan().getOnlineMembers())
        {
            if (player.isClanLeader())
            {
                for (IntIntHolder reward : Config.LEADER_REWARD_WINNER_SIEGE_CLAN)                                    
                    player.addItem("", reward.getId(), reward.getValue(), player, true);    
            }
            else
            {
                for (IntIntHolder reward : Config.REWARD_WINNER_SIEGE_CLAN)                                    
                    player.addItem("", reward.getId(), reward.getValue(), player, true);                    
            } 
        }
        
        return _clan;
    }
    

After finishing the siege, I made a restriction of the same IP plus the 2 players of the same ip wins the item, what did I do wrong?

 

method created for SiegeEnd ()

 

        // Reward Clan winner
        getRewardClanWinner();
        
    private static final Map<String, Integer> _playerIps = new ConcurrentHashMap<>();
    
    public void getRewardClanWinner()
    {
        for (Player player : World.getInstance().getPlayers())
        {
            String pIp = player.getClient().getConnection().getInetAddress().getHostAddress();

            if (!_playerIps.containsKey(pIp))
            {
                _playerIps.put(pIp, 1);
                player.rewardClanWinner().setCastle(getCastle().getOwnerId());
            }
            else
            {
                int count = _playerIps.get(pIp);

                if (count < 1)
                {
                    _playerIps.remove(pIp);
                    _playerIps.put(pIp, count + 1);
                    player.rewardClanWinner().setCastle(getCastle().getOwnerId());
                }
                else
                    player.sendMessage("Already 1 character(s) of your ip have been rewarded, so this character won't be rewarded.");
            }
        }
    }
    

It does not only reward one as I write in the code.

[Tryskell]

	public void getRewardClanWinner()
	{
		final List<String> playerIps = new ArrayList<>();
		
		for (Player player : World.getInstance().getPlayers())
		{
			final String pIp = player.getClient().getConnection().getInetAddress().getHostAddress();
			
			if (!playerIps.contains(pIp))
			{
				playerIps.add(pIp);
				player.rewardClanWinner();
			}
		}
	}

• I don't think it's needed to say to someone who tried to exploit, he exploited. Simply avoid the behavior.
• ArrayList because second parameter got no use.
• No concurrency because no needs to remove or delete in same time than add, and only one iteration is called.
• Inner container instead of class, because no use to keep retained info. Always keep variables in lowest scope possible.

 

Another thing, you should iterate MEMBERS of WINNING CLAN instead of World.getInstance().getPlayers() and rework rewardClanWinner accordingly. The easiest way is to add parameter Clan into getRewardClanWinner, and use it inside the method instead of World.getInstance().getPlayers() .

 

Another thing, you don't reward OFFLINE members. No clue why you wouldn't, since all ONLINE members probably didn't participate to siege. So you have to either keep track about who participated, or reward everyone (OFFLINE included).

Edited September 2, 2019 by Tryskell

[Williams]

7 hours ago, Tryskell said:

	public void getRewardClanWinner()
	{
		final List<String> playerIps = new ArrayList<>();
		
		for (Player player : World.getInstance().getPlayers())
		{
			final String pIp = player.getClient().getConnection().getInetAddress().getHostAddress();
			
			if (!playerIps.contains(pIp))
			{
				playerIps.add(pIp);
				player.rewardClanWinner();
			}
		}
	}

• I don't think it's needed to say to someone who tried to exploit, he exploited. Simply avoid the behavior.
• ArrayList because second parameter got no use.
• No concurrency because no needs to remove or delete in same time than add, and only one iteration is called.
• Inner container instead of class, because no use to keep retained info. Always keep variables in lowest scope possible.

 

Another thing, you should iterate MEMBERS of WINNING CLAN instead of World.getInstance().getPlayers() and rework rewardClanWinner accordingly. The easiest way is to add parameter Clan into getRewardClanWinner, and use it inside the method instead of World.getInstance().getPlayers() .

 

Another thing, you don't reward OFFLINE members. No clue why you wouldn't, since all ONLINE members probably didn't participate to siege. So you have to either keep track about who participated, or reward everyone (OFFLINE included).

 

I will do what you said, thanks. I'm your fan