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

Add debug logging to score module #479

Merged
merged 2 commits into from
Aug 27, 2023
Merged

Add debug logging to score module #479

merged 2 commits into from
Aug 27, 2023

Conversation

nirs
Copy link
Member

@nirs nirs commented Aug 26, 2023

Looks like the documentation does not describe the rules of the game correctly. Add debug logs to make it easier to understand the scoring loop and its semantics.

To see the new logs we need to enable debug level logging. In the test this can be done like this:

pytest --log-cli-level=debug

Example logs from the collisions tests:

rose/server/score_test.py::TestCollisions::test_player_in_lane_wins 
-------------------------------- live log call ---------------------------------
INFO     score:score.py:113 process_actions: name=A lane=0 pos=1,5 score=10 response_time=1.000000
DEBUG    score:score.py:87 player B picked up penguin: got 10 points
INFO     score:score.py:113 process_actions: name=B lane=1 pos=1,6 score=20 response_time=1.000000
PASSED                                                                   [ 94%]
rose/server/score_test.py::TestCollisions::test_after_turn_to_not_in_lane 
-------------------------------- live log call ---------------------------------
DEBUG    score:score.py:26 player B moved left to 2,5
INFO     score:score.py:113 process_actions: name=A lane=0 pos=2,5 score=10 response_time=1.000000
INFO     score:score.py:103 player B collision at 2,5
INFO     score:score.py:113 process_actions: name=B lane=1 pos=2,6 score=0 response_time=1.000000
PASSED                                                                   [ 96%]
rose/server/score_test.py::TestCollisions::test_move_left_out_of_world 
-------------------------------- live log call ---------------------------------
DEBUG    score:score.py:33 player B moved right to 1,8
INFO     score:score.py:113 process_actions: name=A lane=0 pos=1,8 score=10 response_time=1.000000
INFO     score:score.py:103 player B collision at 1,8
INFO     score:score.py:113 process_actions: name=B lane=1 pos=0,8 score=0 response_time=1.000000
PASSED                                                                   [ 98%]
rose/server/score_test.py::TestCollisions::test_move_right_out_of_world 
-------------------------------- live log call ---------------------------------
DEBUG    score:score.py:26 player B moved left to 0,8
INFO     score:score.py:113 process_actions: name=A lane=0 pos=0,8 score=10 response_time=1.000000
INFO     score:score.py:103 player B collision at 0,8
INFO     score:score.py:113 process_actions: name=B lane=1 pos=1,8 score=0 response_time=1.000000
PASSED                                                                   [100%]

@yaacov
Copy link
Member

yaacov commented Aug 27, 2023

LGTM

@nirs
Copy link
Member Author

nirs commented Aug 27, 2023

@yaacov this conflicts with #476 - if it is ready we can merge them first and I will update this pr.

@yaacov
Copy link
Member

yaacov commented Aug 27, 2023

#476 works for me (TM) so I'm ok with merging it first, or after this one :-)

@yaacov
Copy link
Member

yaacov commented Aug 27, 2023

btw// #472 touches the same files is not ready but it won first place in Nitanim :-)
it will be nice to fix it and merge it too ...

@nirs
Copy link
Member Author

nirs commented Aug 27, 2023

btw// #472 touches the same files is not ready but it won first place in Nitanim :-) it will be nice to fix it and merge it too ...

#472 needs more work, do you want to review it?

@yaacov
Copy link
Member

yaacov commented Aug 27, 2023

btw// #472 touches the same files is not ready but it won first place in Nitanim :-) it will be nice to fix it and merge it too ...

#472 needs more work, do you want to review it?

I added my reviews

@nirs hi, #472 is a PR from the NItanim competition, it has no "owner" now. the kids that made it finished the program and will not continue with there PRs, ( even if they want to continue, they will not know how to respond to the review or how to fix the PR ).

This PRs needs someone with permissions to edit PRs, fix it, and merge it (don't wait for the kids to fix it).

We can turn now debug logs and watch the log to understand how scoring
works during the game. This can be useful to debug the code and the
tests and improving the docs.
test_player_in_lane_wins() is wrong, not testing any collision. We need
to fix it before we do that fix the comments to describe whats going on
correctly.
@nirs
Copy link
Member Author

nirs commented Aug 27, 2023

@yaacov logging is much better after to your changes.

There are some issue we can fix later because we may change player.x or player.y to invalid value and log the invalid value before the value is clipped to valid value.

Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

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

lgtm

@nirs nirs merged commit 117d640 into RedHat-Israel:master Aug 27, 2023
1 check passed
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.

None yet

2 participants