Unable to detect an incoming trade

[Hayase]

Player worker = trade.getLastRequestingPlayer();
log("Waiting for a trade");
if (worker != null) { //this is broken
    if (worker.isVisible()) {
        log("I see your trade! Allow me...");
        if (worker.interact("Trade with")) {
            new ConditionalSleep(5000) {
                @[member='Override']
                public boolean condition() throws InterruptedException {
                    return trade.isCurrentlyTrading();
                }
            }.sleep();
        }
        if (trade.isCurrentlyTrading())
            //do stuff
    } else {
        log("I CAN'T SEE YOU!");
        camera.toEntity(worker);
    }
} else {
    log("Where's the trader?");
}

Currently the script doesn't detect a trade. It always returns null for all incoming trades. What is the proper usage to getLastRequestingPlayer()?

Edited December 20, 2016 by Hayase

[Saiyan]

use onMessage

[Hayase]

use onMessage

 

Do I need to do anything special with onMessage? I created a function checkForTrade() using the original code and I'm not sure if it's working or not.

@[member='Override']
public void onMessage(Message c) {
    String m = c.getMessage().toLowerCase();
    checkForTrade();
    if (trade.getLastRequestingPlayer() !=null && m.contains(trade.getLastRequestingPlayer().getName().toLowerCase())) {
        log("We found a trade in our onMessage!");
    }
}

Seems to be working now, thanks Saiyan.

Edited December 20, 2016 by Hayase

[Abuse]

That seems to be correct

 

Try to store a seperate boolean "tradeRequested" and set it to true in the onMessage function, and back to false when you complete a trade / fail to trade x amount of times.

 

It's bad practice to have blocking methods in the onMessage handler

[liverare]

Here's an untested and off-the-cuff example of how I'd go about handling this:

	/*
	 * Concurrent hash map is more suitable, because we're going to read and
	 * write to it from different threads that can run in parallel
	 */
	Map<String, Long> tradeRequests = new ConcurrentHashMap<>();
	
	@[member=Override]
	public int onLoop() throws InterruptedException {
		
		Player worker = null;
		
		// Remove all old requests
		removeOldTradeRequests(15000); // 15 seconds
		
		// Check to see if we've been traded
		if (hasBeenTradedBy("whoever")) {
			
			// Now find that actual player in-game
			worker = players.closest("whoever");
			
			// Check worker is valid
			if (worker != null && worker.exists()) {
				
				// Trade worker back
				worker.interact("Trade");
			}
		}
		
		return 0;
	}

	/*
	 * In here we're simply going to capture all trade requests and store them.
	 * 
	 * Because we're dealing with a hash map, old trade requests will be
	 * overwritten by the newer ones, so we know who traded and when, to help
	 * filter out the old and irrelevant ones.
	 */
	@[member=Override]
	public void onMessage(Message message) throws InterruptedException {
		
		// Basic assertion
		assert (message != null);
		
		// Check message type
		// Check message suffix
		if (message.getType() == Message.MessageType.GAME
				&& message.getMessage().endsWith(" wishes to trade with you.")) {
			
			// Store result
			tradeRequests.put(message.getUsername(), message.getTime());
		}
	}
	
	public final synchronized boolean hasBeenTradedBy(String userName) {
		
		return tradeRequests.containsKey(userName);
	}
	
	/**
	 * Remove all old trade requests from hash map
	 */
	public final synchronized void removeOldTradeRequests(long expirationInMilliseconds) {
		
		final long now = System.currentTimeMillis();
		
		tradeRequests.forEach((userName, timeStamp) -> {
			
			if ((now - timeStamp) >= expirationInMilliseconds) {
				
				tradeRequests.remove(userName);
			}
		});
	}

[Abuse]

 

Here's an untested and off-the-cuff example of how I'd go about handling this:

	/*
	 * Concurrent hash map is more suitable, because we're going to read and
	 * write to it from different threads that can run in parallel
	 */
	Map<String, Long> tradeRequests = new ConcurrentHashMap<>();
	
	@[member='Override']
	public int onLoop() throws InterruptedException {
		
		Player worker = null;
		
		// Remove all old requests
		removeOldTradeRequests(15000); // 15 seconds
		
		// Check to see if we've been traded
		if (hasBeenTradedBy("whoever")) {
			
			// Now find that actual player in-game
			worker = players.closest("whoever");
			
			// Check worker is valid
			if (worker != null && worker.exists()) {
				
				// Trade worker back
				worker.interact("Trade");
			}
		}
		
		return 0;
	}

	/*
	 * In here we're simply going to capture all trade requests and store them.
	 * 
	 * Because we're dealing with a hash map, old trade requests will be
	 * overwritten by the newer ones, so we know who traded and when, to help
	 * filter out the old and irrelevant ones.
	 */
	@[member='Override']
	public void onMessage(Message message) throws InterruptedException {
		
		// Basic assertion
		assert (message != null);
		
		// Check message type
		// Check message suffix
		if (message.getType() == Message.MessageType.GAME
				&& message.getMessage().endsWith(" wishes to trade with you.")) {
			
			// Store result
			tradeRequests.put(message.getUsername(), message.getTime());
		}
	}
	
	public final synchronized boolean hasBeenTradedBy(String userName) {
		
		return tradeRequests.containsKey(userName);
	}
	
	/**
	 * Remove all old trade requests from hash map
	 */
	public final synchronized void removeOldTradeRequests(long expirationInMilliseconds) {
		
		final long now = System.currentTimeMillis();
		
		tradeRequests.forEach((userName, timeStamp) -> {
			
			if ((now - timeStamp) >= expirationInMilliseconds) {
				
				tradeRequests.remove(userName);
			}
		});
	}

 

I don't think that you need to synchronzie onMessage() as it's always called before (or after, cant remember) the onLoop(), it's on the same thread.

[liverare]

I don't think that you need to synchronzie onMessage() as it's always called before (or after, cant remember) the onLoop(), it's on the same thread.

 

Good to know. I'm not sure myself, so better safe than sorry!

[Abuse]

Good to know. I'm not sure myself, so better safe than sorry!

 

Agreed Found out the hard way, when I was trying to reading messages while doing other actions, they all arrived suspiciously late

[Hayase]

 

Here's an untested and off-the-cuff example of how I'd go about handling this:

	/*
	 * Concurrent hash map is more suitable, because we're going to read and
	 * write to it from different threads that can run in parallel
	 */
	Map<String, Long> tradeRequests = new ConcurrentHashMap<>();
	
	@[member='Override']
	public int onLoop() throws InterruptedException {
		
		Player worker = null;
		
		// Remove all old requests
		removeOldTradeRequests(15000); // 15 seconds
		
		// Check to see if we've been traded
		if (hasBeenTradedBy("whoever")) {
			
			// Now find that actual player in-game
			worker = players.closest("whoever");
			
			// Check worker is valid
			if (worker != null && worker.exists()) {
				
				// Trade worker back
				worker.interact("Trade");
			}
		}
		
		return 0;
	}

	/*
	 * In here we're simply going to capture all trade requests and store them.
	 * 
	 * Because we're dealing with a hash map, old trade requests will be
	 * overwritten by the newer ones, so we know who traded and when, to help
	 * filter out the old and irrelevant ones.
	 */
	@[member='Override']
	public void onMessage(Message message) throws InterruptedException {
		
		// Basic assertion
		assert (message != null);
		
		// Check message type
		// Check message suffix
		if (message.getType() == Message.MessageType.GAME
				&& message.getMessage().endsWith(" wishes to trade with you.")) {
			
			// Store result
			tradeRequests.put(message.getUsername(), message.getTime());
		}
	}
	
	public final synchronized boolean hasBeenTradedBy(String userName) {
		
		return tradeRequests.containsKey(userName);
	}
	
	/**
	 * Remove all old trade requests from hash map
	 */
	public final synchronized void removeOldTradeRequests(long expirationInMilliseconds) {
		
		final long now = System.currentTimeMillis();
		
		tradeRequests.forEach((userName, timeStamp) -> {
			
			if ((now - timeStamp) >= expirationInMilliseconds) {
				
				tradeRequests.remove(userName);
			}
		});
	}

 

Very elegant documentation it was a pleasure to read