Discourage camping: Food relocation and ghost shadow

Debilski commented 1 month ago

Draft PR to relocate food after it has sat 60 turns next to a friendly bot.

TODO:

[x] Relocation algorithm contains a while True loop that is potentially not halting when no free space is found
[x] Only show food lifetime in UI debug mode
[x] More extensive testing, especially of the relocation part
[ ] The use of the game_state could be refactored (analogous to noiser() we should only use the items that are needed from the game_state
[x] Testing with legacy bots to see what breaks (with different values for lifetime and range)
[x] I currently relocate to any location in the home zone. We could decide to only relocate to a spot outside the bot shadow(s)
[ ] Needs to be documented!
[x] There may be bugs
[ ] The code does not create any ‘events’ yet. Would be interesting to have a statistics at the end of the game of how much food was relocated
[x] A bunch of old tests may fail now because a StoppingPlayer on a small layout (or even a big one) will now move the food around
[x] I suppose food should not relocate underneath an enemy bot, this needs to be fixed

Closes #810

Fun thing I noted: Following a bot inside the home zone may have the fun effect of removing a food that the bot is just about to eat. :)

otizonaizit commented 1 month ago

are you open to contributions? @pberkes and me were working on it already in the branch https://github.com/ASPP/pelita/tree/nocamp . We have a better solution than the infinite loop for food relocation I think. Also, after testing it seems that the radius of 5 is a bit too big, 3 seems a better compromise. I did not understand how you deal with food which is within the shadow of both bots. Anyway, if you are open for contributions I can have a look tonight and propose some improvements.

otizonaizit commented 1 month ago

btw, I wouldn't call it squatting. The technical term in videogames is camping, no? So better use that. I can take care of the renaming too.

otizonaizit commented 1 month ago

... another thing: in our attempt we decided to relocate food outside of the radius and only on free squares (i.e. no on enemy or partner bots) and not on the border (like we do when generating the mazes).

Debilski commented 1 month ago

Ah, I didn’t see. Yeah, please add the new approach.

I check with any whether any of the bots throws a shade and only one lifetime is removed no matter how many bots.

Name can be changed. Honestly I had a blackout and forgot our name. (But is ‘camping’ really the official term?)

otizonaizit commented 1 month ago

I have addressed the most pressing things. Left to be done is user documentation on pelita_template and a better idea for the GUI. I thought that in the non-debug view we could let the food become gray when only 1 round lifetime is left? In the debug view I am not sure it is useful to show the lifetime of all pellets, maybe we could just show it for those that are within a distance of LIFETIME_DISTANCE, which is now different from NOISE_RADIUS

Debilski commented 1 month ago

The problem with opening this up for additions is of course that I never know when you’re finished … :)

otizonaizit commented 1 month ago

I pondered about it a lot. If you do it at every turn you have the problem that food pellets which are located in the shadow of two bots expire at twice the speed than the others. You can do it only once per round, but then the blue team has a slight advantage, because they can sit there for (in the worst case) 4 turns longer than the opponent.

If you want to do it at every turn we need to keep track of the double shadowing and I couldn't come up with a way to do it that is not too complicated.

I thought this is a good compromise. Once per round, but as soon as possible.

On Thu 01 Aug, 11:59 +0000, Rike-Benjamin Schuppner @.***> wrote:

@Debilski commented on this pull request.

––––––––––

In pelita/game.py¹:

Now update the round counter

game_state.update(next_round_turn(game_state))

turn = game_state['turn'] round = game_state['round'] team = turn % 2

if turn >= 2:

I don’t understand why this is needed. The code should remove only one lifetime unit per turn. I would prefer having it either completely symmetric (in every turn and with lifetime x 4) or perhaps after turn == 3 and then do it for both teams at the same time.

— Reply to this email directly, view it on GitHub², or unsubscribe³. You are receiving this because you commented.☘Message ID: @.***>

––––

¹ https://github.com/ASPP/pelita/pull/811#discussion_r1700679978 ² https://github.com/ASPP/pelita/pull/811#pullrequestreview-2213664612 ³ https://github.com/notifications/unsubscribe-auth/AACUYCYNP7IO3TKDCJOPWG3ZPKARTAVCNFSM6AAAAABLYUAYSGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMJTGY3DINRRGI

Debilski commented 1 month ago

I pondered about it a lot. If you do it at every turn you have the problem that food pellets which are located in the shadow of two bots expire at twice the speed than the others.

But how? The code does not do that. (I am really not sure if I’m just misunderstanding you here.)Remove the if turn: and play

pelita --layout food_expiry.layout --stop-at 0 pelita/player/StoppingPlayer.py pelita/player/StoppingPlayer.py

with

cat food_expiry.layout 

################
#a..       ...y#
#..######.. ..x#
#...  .   .  #.#
#.#  .   .  . .#
#.#  ..###### .#
#b ..       .. #
################

Then play stepwise. All food expires at the same rate (or not at all) no matter how many bots are close.

And if this were the case that two bots speed up the exipry then of course the if turn: check would not do anything against it.

otizonaizit commented 1 month ago

I pondered about it a lot. If you do it at every turn you have the problem that food pellets which are located in the shadow of two bots expire at twice the speed than the others.

But how? The code does not do that. (I am really not sure if I’m just misunderstanding you here.)Remove the if turn: and play

arghhh, you are right. I fixed it implicitly while refactoring it. But you still agree that it makes more sense to make the update at every turn but only for the team that moves, right? So we just increase MAX_FOOD_LIFETIME*2? Or you want to update at every turn for everyone?

Debilski commented 1 month ago

and a better idea for the GUI. I thought that in the non-debug view we could let the food become gray when only 1 round lifetime is left? In the debug view I am not sure it is useful to show the lifetime of all pellets, maybe we could just show it for those that are within a distance of LIFETIME_DISTANCE, which is now different from NOISE_RADIUS

Makes sense. The current UI is more of a help for development.

Debilski commented 1 month ago

I pondered about it a lot. If you do it at every turn you have the problem that food pellets which are located in the shadow of two bots expire at twice the speed than the others. But how? The code does not do that. (I am really not sure if I’m just misunderstanding you here.)Remove the if turn: and play arghhh, you are right. I fixed it implicitly while refactoring it. But you still agree that it makes more sense to make the update at every turn but only for the team that moves, right? So we just increase MAX_FOOD_LIFETIME*2? Or you want to update at every turn for everyone?

I don’t know what you ‘fixed’ but my version (git checkout 268f50f) worked just the same. :)

Doing it only for the current team (but otherwise in each step) is fine with me. Should make the code simpler.

otizonaizit commented 1 month ago

I don’t know what you ‘fixed’ but my version (git checkout 268f50f) worked just the same. :) I fixed mine and Pietro's version, which I was refactoring while cleaning your version...

Doing it only for the current team (but otherwise in each step) is fine with me. Should make the code simpler.

Good, so the fix is simple. Should I do it or you do it?

otizonaizit commented 1 month ago

Regarding the GUI and the docs: who does what? I think it's better to coordinate ;-)

otizonaizit commented 1 month ago

Btw, now that I made it configurable, should we add a CLI switch to allow camping? And for setting the parameters? Or is this overkill?

otizonaizit commented 1 month ago

I don’t know what you ‘fixed’ but my version (git checkout 268f50f) worked just the same. :) I fixed mine and Pietro's version, which I was refactoring while cleaning your version...

Doing it only for the current team (but otherwise in each step) is fine with me. Should make the code simpler.

Good, so the fix is simple. Should I do it or you do it?

done

Debilski commented 1 month ago

I think CLI to allow camping is fine (for compatibility with old versions). Parameters maybe only when the need arises. (Not sure if you missed it, but I already included a CLI switch still named --allow-squatting.)

I’ll revert the turn check now (ok too late) and do some minor refactorings (maybe) tomorrow with the filters code when I can study it more in detail and I can take care of the GUI in the next days (do we need to show the shadow also?). The GUI is already messy and needs a cleanup here and there. If you want to do some docs, that would be good.

otizonaizit commented 1 month ago

I think CLI to allow camping is fine (for compatibility with old versions). Parameters maybe only when the need arises. (Not sure if you missed it, but I already included a CLI switch still named --allow-squatting.) yep, I missed it. But it doesn't do anything at the moment, right? Will you fix it?

I’ll revert the turn check now (ok too late) and do some minor refactorings (maybe) tomorrow with the filters code when I can study it more in detail and I can take care of the GUI in the next days (do we need to show the shadow also?). The GUI is already messy and needs a cleanup here and there. If you want to do some docs, that would be good.

I think that showing the shadow (in light grey) is useful, but only when it's the team turn to move and only for the bots in their homezone. For the pellets within the shadow we can then show the remaining lifetime. For the non-debug mode, just the greying out of the pellets. I am not sure for how many turns the greying out should persist. Only one turn is not enough to perceive it.

OK, so I will add the docs to pelita_template. Once we have the GUI I'd like to add a screenshot of the shadow too. You do the rest, OK? So I won't push to this branch anymore, happy? :-)

otizonaizit commented 1 month ago

another thing: I am sure people are going to ask how to detect if a food pellet is expiring. Could you make the dictionary food_lifetime available as an attribute on the Bot object? It could be used to locate an enemy bot with more precision than using the noisy bot.position, but if they figure out how to do that they deserve to use it. I am going to add it already to the documentation, unless you complain very loudly.

Debilski commented 1 month ago

Hmm, I don‘t know. I am happy to add it for one’s own food (although we might simply add a function shaded_food() that could then return the food pellets that are in danger of relocating and let the user do the counting). The main idea has been to make camping less attractive not to bring in a complete new game element. In the sense that: if you do not camp, then the game will play as it used to. This change would somewhat immediately devaluate (at least a little) all bots that do not use this new information.

We could only hand out the info for only the food inside the noise radius though. :)

That said, I am not strictly against it but I also wouldn’t mind bringing this in as a new feature gradually at a later point (if people really want to have it).

Opinion scores:

Give out lifetimes for one’s own food: +0.5
Give out lifetimes for food inside the noise radius: +0.2
Give out lifetimes for all food: -0.1

Just like with noise, I think there is a danger of people overthinking this (I can see the intro talk becoming 10 minutes longer already) so maybe less info is more: Food moves. Deal with it. :)

Btw: should we change it such that only ghosts throw a shadow? (With the shorter radius this is not really relevant anymore, but a point could be made that only ghosts should be penalised.)

otizonaizit commented 1 month ago

Hmm, I don‘t know. I am happy to add it for one’s own food (although we might simply add a function shaded_food() that could then return the food pellets that are in danger of relocating and let the user do the counting). The main idea has been to make camping less attractive not to bring in a complete new game element. In the sense that: if you do not camp, then the game will play as it used to. This change would somewhat immediately devaluate (at least a little) all bots that do not use this new information.

yes, you are right.

We could only hand out the info for only the food inside the noise radius though. :)

That said, I am not strictly against it but I also wouldn’t mind bringing this in as a new feature gradually at a later point (if people really want to have it).

That's the best option, yes. Let's start by only publishing bot.shaded_food and bot.other.shaded_food (I would argue for it to be a list though, not a method). If the bots are not in their homezone the lists are empty, right? The two corresponding lists on bot.enemy[0,1] will be always empty, what do you think?

Let's see if publishing this info on food pushes people to create ugly and buggy self-written counting routines.

Just like with noise, I think there is a danger of people overthinking this (I can see the intro talk becoming 10 minutes longer already) so maybe less info is more: Food moves. Deal with it. :)

yes, agreed.

Btw: should we change it such that only ghosts throw a shadow? (With the shorter radius this is not really relevant anymore, but a point could be made that only ghosts should be penalised.)

Not sure what you mean here? The code already only casts a shadow for ghosts (i.e. only food expires which is in your own homezone). Or do you mean something else completely?

Debilski commented 1 month ago

We could only hand out the info for only the food inside the noise radius though. :) That said, I am not strictly against it but I also wouldn’t mind bringing this in as a new feature gradually at a later point (if people really want to have it).

That's the best option, yes. Let's start by only publishing bot.shaded_food and bot.other.shaded_food (I would argue for it to be a list though, not a method). If the bots are not in their homezone the lists are empty, right? The two corresponding lists on bot.enemy[0,1] will be always empty, what do you think?

Yeah, sounds good. I can add it to the code later.

Let's see if publishing this info on food pushes people to create ugly and buggy self-written counting routines.

Not unlikely :)

Btw: should we change it such that only ghosts throw a shadow? (With the shorter radius this is not really relevant anymore, but a point could be made that only ghosts should be penalised.)

Not sure what you mean here? The code already only casts a shadow for ghosts (i.e. only food expires which is in your own homezone). Or do you mean something else completely?

Nope, right now both ghosts and pacman throw a shadow. But if you already thought that only ghosts do, then I will change the code. Meaning: Once you move across the border and become a pacman, all food behind you will have the lifetime resetted.

otizonaizit commented 1 month ago

Not sure what you mean here? The code already only casts a shadow for ghosts (i.e. only food expires which is in your own homezone). Or do you mean something else completely?

Nope, right now both ghosts and pacman throw a shadow. But if you already thought that only ghosts do, then I will change the code. Meaning: Once you move across the border and become a pacman, all food behind you will have the lifetime resetted.

Good catch! Yes, it makes sense! We need a test case for this!

Debilski commented 1 month ago

Do you think we need to have all pellets in food_lifetime? I think I’ll change it so that only the food that is shadowed is in there. This keeps the fingerprint of the game state lower (less noisy when printed) and the camping-allowed mode degrades more nicely (food_lifetime is simply always empty instead of having arbitrary numbers in there).

otizonaizit commented 1 month ago

Do you think we need to have all pellets in food_lifetime? I think I’ll change it so that only the food that is shadowed is in there. This keeps the fingerprint of the game state lower (less noisy when printed) and the camping-allowed mode degrades more nicely (food_lifetime is simply always empty instead of having arbitrary numbers in there).

As long as the implementation doesn't get more complicated... we need to maintain it. But sure, why not. In the prototype it felt easier to put everything in there.

Debilski commented 1 month ago

Might get even simpler if we just count up from 0 …

Debilski commented 1 month ago

A few more changes:

I renamed food_lifetime to food_age and start counting from 0. This should make the handling of unshadowed elements a little more natural as we can just query gs['food_age'].get(pos, 0) to get the age of a food pellet
Analogously to food, food_age is now split between the teams. In the end it does not matter much but it feels more correct to align the data here.

Debilski commented 1 month ago

In principle ready to be merged. The remaining open points and parameter fine-tuning may be done from main.

ASPP / pelita

Discourage camping: Food relocation and ghost shadow #811

Now update the round counter