Remove simulation logic from worker manager #760
Conversation
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @NiallEgan)
aimmo-game/simulation/game_runner.py, line 4 at r1 (raw file):
WORKER_UPDATE_SLEEP_TIME = 5
This was 10 before, correct? Are we 100% that this time is fine?
aimmo-game/tests/test_simulation/fake_game_runner.py, line 16 at r1 (raw file):
"" TODO: This class probably will want to be deleted once GameRunner is complete.
I would argue we don't commit TODO's and instead we update PRs and issues with the new tasks. I'd update the acceptance criteria for whatever task this needs to be done in and remove this.
If others disagree or something, change the double quotes to single.
aimmo-game/tests/test_simulation/test_game_runner.py, line 11 at r1 (raw file):
from simulation.game_state import GameState from simulation.game_runner import GameRunner from .concrete_worker_manager import ConcreteWorkerManager
Do we need these explicit relative imports (dots)?
aimmo-game/tests/test_simulation/test_game_runner.py, line 53 at r1 (raw file):
self.mock_communicator.get_game_metadata = mock.MagicMock() self.game_runner.update() # noinspection PyUnresolvedReferences
Why is it unresolved? I think your intelliJ might be set up wrong with root directories. We shouldn't need a noinspection like this without a reason
aimmo-game/tests/test_simulation/test_game_runner.py, line 87 at r1 (raw file):
self.game_runner.update() del self.mock_communicator.data['main']['users'][1] print('Modified data: {}'.format(self.mock_communicator.data['main']['users']))
Do we need this?
aimmo-game/tests/test_simulation/test_game_runner.py, line 88 at r1 (raw file):
del self.mock_communicator.data['main']['users'][1] print('Modified data: {}'.format(self.mock_communicator.data['main']['users'])) print('Workers: {}'.format(self.game_runner.worker_manager.avatar_id_to_worker.keys()))
Do we need this?
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @NiallEgan)
a discussion (no related file):
Sort out codeclimate. Mark them as wontfix or something if they're not valid. Otherwise they need fixing
There was a problem hiding this comment.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @NiallEgan and @OlafSzmidt)
aimmo-game/simulation/game_runner.py, line 4 at r1 (raw file):
Previously, OlafSzmidt (Olaf Szmidt) wrote…
WORKER_UPDATE_SLEEP_TIME = 5This was 10 before, correct? Are we 100% that this time is fine?
Yes it was. Will change back.
aimmo-game/tests/test_simulation/fake_game_runner.py, line 16 at r1 (raw file):
Previously, OlafSzmidt (Olaf Szmidt) wrote…
"" TODO: This class probably will want to be deleted once GameRunner is complete.I would argue we don't commit TODO's and instead we update PRs and issues with the new tasks. I'd update the acceptance criteria for whatever task this needs to be done in and remove this.
If others disagree or something, change the double quotes to single.
Done.
aimmo-game/tests/test_simulation/test_game_runner.py, line 11 at r1 (raw file):
Previously, OlafSzmidt (Olaf Szmidt) wrote…
Do we need these explicit relative imports (dots)?
Not sure... they were here before (in test_worker_manager_. I think the problem is that in some files we do from __future__ import absolute_import, which means different files have different import rules.
aimmo-game/tests/test_simulation/test_game_runner.py, line 53 at r1 (raw file):
Previously, OlafSzmidt (Olaf Szmidt) wrote…
Why is it unresolved? I think your intelliJ might be set up wrong with root directories. We shouldn't need a noinspection like this without a reason
This was in test_worker_manager before. I think there's maybe some issue regarding inspections and mocked objects.
aimmo-game/tests/test_simulation/test_game_runner.py, line 87 at r1 (raw file):
Previously, OlafSzmidt (Olaf Szmidt) wrote…
Do we need this?
Done.
aimmo-game/tests/test_simulation/test_game_runner.py, line 88 at r1 (raw file):
Previously, OlafSzmidt (Olaf Szmidt) wrote…
Do we need this?
Done.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r3.
Reviewable status:complete! all files reviewed, all discussions resolved
This change is