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

ENH: Add axis labels to grid mode. #858

Merged
merged 3 commits into from
Mar 11, 2025
Merged

Conversation

Debilski
Copy link
Member

@Debilski Debilski commented Mar 10, 2025

Labels are not auto-scaled (the margin is also not auto-scaled so it should always fit, unless of course, we have more than 100 cells in one dimension or so).

image

@otizonaizit
Copy link
Member

otizonaizit commented Mar 10, 2025

yes, this is very cool!!!
Why not adding on both sides? It makes it easier to locate a coordinate without touching the screen with your fingers ;-)
Also, I guess that people like @jbdyn may appreciate the axis labels x and y?

@Debilski
Copy link
Member Author

Idk. I think the graphic is already getting overloaded. You can still click a field and get the coordinates shown in the status bar.
x and y are the red bot names. This will be confusing (plus the coordinates are never referred to by x or y, it is all tuples).

@otizonaizit
Copy link
Member

otizonaizit commented Mar 10, 2025

Idk. I think the graphic is already getting overloaded. You can still click a field and get the coordinates shown in the status bar. x and y are the red bot names. This will be confusing (plus the coordinates are never referred to by x or y, it is all tuples).

what was confusing for @jbdyn and for many others in the past is that when you look at a coordinate (10, 4) you don't know if 10 refers to the horizontal or the vertical dimension. That ambiguity is still there, that is what I meant by marking with x and y. I don't think it is confusing that the bots have the same names. Writing index 0 and index 1 instead may be more precise, but I doubt it would be less confusing. People are very much used to see coordinates expressed as (x, y). Just look at our own code: we use it all the time.

@Debilski
Copy link
Member Author

image

@jbdyn
Copy link

jbdyn commented Mar 11, 2025

Also, I guess that people like @jbdyn may appreciate the axis labels x and y?

I certainly do. 😉 Thanks @Debilski!

For reference, that confuses me:

cartesian coordinates x ⬆️ y ➡️
matrix indices i ⬇️ j ➡️
image coordinates x ➡️ y ⬇️

@jbdyn
Copy link

jbdyn commented Mar 11, 2025

I think the graphic is already getting overloaded.

How about fixing the number of ticks and set the number only there? Like 5, 10, 15, ...

Also, the axis labels are the additions which improve the UI for me the most. I image that this labels are better displayed at the origin in the top left as this is how we read text:

     x                   5
 y |   |   |   |   |   |   |   |
   |   |   |   |   |   |   |   |
   |   |   |   |   |   |   |   |
   |   |   |   |   |   |   |   |
   |   |   |   |   |   |   |   |
 5 |   |   |   |   |   |   |   |
   |   |   |   |   |   |   |   |

x and y are the red bot names.

Right, that will be confusing. So row and column for the coordinates maybe? Or b1, b2, r1 and r2 for the bots?

@Debilski
Copy link
Member Author

I guess only having every 5th tick would also work but I can also live with having all numbers there as long as we don’t clutter it any more.

The best way to improve things in my opinion is to make this connection clearer that I marked in the graphics. In my mind this should resolve all uncertainties better than any axis label could.

image

Suggestion with axis label instead of 0:

image

@otizonaizit
Copy link
Member

x and y are the red bot names.

Right, that will be confusing. So row and column for the coordinates maybe? Or b1, b2, r1 and r2 for the bots?

The bot names need to be one ASCII character, otherwise we can't represent them in a layout string. So that rules out b1 etc. We had a very long discussion about bot names in the past, and a different convention already (0, 1, 2, 3). The current one seems to be the one that confuses the least people, so I wouldn't want to touch it again, to be honest.
Also, if we touch it, we break the API, i.e. we break all old bots. And, unfortunately I have seen a lot of code depending on the bot name. In this year's TU course a group was choosing behaviour depending on the bot name being a or x, as opposed to b or y.

@otizonaizit
Copy link
Member

otizonaizit commented Mar 11, 2025

I guess only having every 5th tick would also work but I can also live with having all numbers there as long as we don’t clutter it any more.

I sincerely do not understand the argument about not "cluttering" the UI. We are talking about the "debug" option. The more info we have there the better. The way I see people using that option is in combination with --stop-at. They then switch "debugging" on and try to understand what happens. That is also why I really prefer having all the number labels for the axis and not only every 5th.

The best way to improve things in my opinion is to make this connection clearer that I marked in the graphics. In my mind this should resolve all uncertainties better than any axis label could.

I can tell you why even for me that know that connection by heart, it still does not cut it.
I print a list of coordinates from my bot, for example some kind of danger zone around the enemy. I see (13, 4). Now I want to locate that tile in the UI. What Jakob and me were doing the other night was frantically clicking around trying to find it. With the number labels and the axis labels we wouldn't have needed to click anywhere. Or even better, we would click only once just to confirm that we located the right tile.

So, the connection you are talking about is pretty important, and it is documented already. We just need to show it to people when they program and debug. The new labels are solving a different problem and they do it pretty well. I'd put x and y at the origin, and, as I said, I'd put the numbers also on the right and bottom axis.

@Debilski Debilski merged commit bfcbd5f into ASPP:main Mar 11, 2025
33 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 11, 2025
@Debilski Debilski deleted the feature/gridlabels branch March 11, 2025 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants