Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robot activity counts in F2 menu #1484

Merged
merged 23 commits into from
Sep 7, 2023
Merged

Robot activity counts in F2 menu #1484

merged 23 commits into from
Sep 7, 2023

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Aug 31, 2023

Towards #1341.

scripts/play.sh -i data/scenarios/Testing/1341-command-count.yaml --autoplay --speed 1

image

@kostmo kostmo force-pushed the feature/command-count branch 4 times, most recently from 77783eb to 7d246ec Compare September 4, 2023 06:05
@kostmo kostmo marked this pull request as ready for review September 4, 2023 06:05
@kostmo kostmo added the CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. label Sep 4, 2023
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks cool! 🚀 Also, thanks @kostmo for the detailed documentation 👍

I have not yet read through all of it, but I will try to find time to do so this week and also test this out on the scenarios.

src/Swarm/Game/Step.hs Outdated Show resolved Hide resolved
src/Swarm/Util/WindowedCounter.hs Outdated Show resolved Hide resolved
src/Swarm/Util/WindowedCounter.hs Outdated Show resolved Hide resolved
src/Swarm/TUI/View.hs Show resolved Hide resolved
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good! I think this will give a lot of useful information when debugging robot programs. I think there are just a few more things to clean up / resolve before merging.

  • Do you have any idea why idler1 only ever gets to 98.4% Activity? Seems to me like it ought to be 100%.
    • Edited to add: I think it's because the gameTick function runs all robots, then increments the current tick. So at the time we are rendering a frame, the current tick will always be strictly greater than any ticks stored in the WindowedCounter for any robot; hence getOccupancy will never be 1.
  • We should probably add a wiki page somewhere that explains in detail what each of the columns means, and link to it from somewhere (e.g. the bottom of the Robots panel). In particular I imagine many players will be confused about the difference between actions, commands, and cycles. I don't care whether we do that in this PR or split it out into another issue and do it later.

src/Swarm/Util/WindowedCounter.hs Outdated Show resolved Hide resolved
src/Swarm/TUI/View.hs Outdated Show resolved Hide resolved
src/Swarm/Util/WindowedCounter.hs Outdated Show resolved Hide resolved
src/Swarm/Util/WindowedCounter.hs Outdated Show resolved Hide resolved
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Sep 7, 2023
@mergify mergify bot merged commit 1aa92d0 into main Sep 7, 2023
10 checks passed
@mergify mergify bot deleted the feature/command-count branch September 7, 2023 06:00
mergify bot pushed a commit that referenced this pull request Oct 1, 2023
Extension of #1484 that partially mitigates #1558.

Let's not bother to track the activity levels of system robots when we're not in creative mode, because they won't even show up in the `F2` dialog.  This mitigation can be substantial, as we generally expect there to be many more system robots than player robots.

It doesn't have quite the same "warm up" drawback as the potential mitigation described in #1558 (comment), because player robots will always be warmed up, and system robots will have been warmed up if we've been playing in creative mode.  We wouldn't necessarily expect frequent toggling in-and-out of creative mode while trying to observe performance in the `F2` dialog.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants