Closed lpassey closed 3 years ago
Thank you again! Looks good; I'll merge this tonight.
Your improvement to JSettlers is now merged! Because the bot already pauses 3 seconds in handleMAKEOFFER, I ended up subtracting 3 seconds for the second pause so the parameter sets the total time.
Thanks again for patiently working with me to get this done.
Hello Lee,
Thanks for taking the time to rework this. In this PR it's much easier to see the code you've worked on.
This is a good contribution. It's almost perfect; I have 2 requests please:
Instead of SOCRobotBrain.considerOffer, please add your code in handleMAKEOFFER near the existing pause (currently 3.5 seconds). I've committed a check there after the pause to make sure the offer is still valid. That doesn't totally cover your todo about reacting to other trade responses/interrupting the long pause, but it should help prevent sending out-of-date responses. I'll ponder what else we can do about reacting.
To keep things simple, we don't need to go through SOCUpdateRobotParams to convey this to the bots. For consistency I think it makes sense to follow the same path as PROP_JSETTLERS_BOTS_FASTPAUSEPERCENT: SOCServer.initSocServer reads that property and sets the corresponding SOCRobotBrain field BOTS_ONLY_FAST_PAUSE_FACTOR. So initSocServer would set a new SOCRobotBrain static field, handleMAKEOFFER would use it.
I think that gives us the most straightforward approach, but I don't mind discussing it. If it sounds good enough to you as-is, I'm happy to accept updated code.
Thanks again, -Jeremy