fix: load correct worksheet in aimmo-game #1476
Conversation
There was a problem hiding this comment.
Did the pip files change just because of the python version update to 3.7 or is there something else?
Reviewed 1 of 9 files at r1.
Reviewable status: 1 of 9 files reviewed, 2 unresolved discussions (waiting on @razvan-pro)
aimmo-game/simulation/avatar/avatar_manager.py, line 13 at r1 (raw file):
WorksheetData = None
Might be opening a can of worms, but I like the convention of no space for default parameter. What do you think?
aimmo-game/simulation/worksheet/init.py, line 2 at r1 (raw file):
from .worksheet import WorksheetData __all__ = ["WorksheetData"]
can we empty this file?
There was a problem hiding this comment.
Reviewed 3 of 9 files at r1.
Reviewable status: 4 of 9 files reviewed, 2 unresolved discussions (waiting on @razvan-pro)
There was a problem hiding this comment.
The one in aimmo-game yes. For the other I ran pipenv lock to update it 🙂
Reviewable status: 4 of 9 files reviewed, 2 unresolved discussions (waiting on @dionizh and @razvan-pro)
aimmo-game/simulation/avatar/avatar_manager.py, line 13 at r1 (raw file):
Previously, dionizh (Dioni Zhong) wrote…
WorksheetData = NoneMight be opening a can of worms, but I like the convention of no space for default parameter. What do you think?
Formatted with black which seems to like spaces 🙂 I'll go with black 🙂
aimmo-game/simulation/worksheet/init.py, line 2 at r1 (raw file):
Previously, dionizh (Dioni Zhong) wrote…
from .worksheet import WorksheetData __all__ = ["WorksheetData"]can we empty this file?
Technically yes, but then we'd have to import from simulation.worksheet.worksheet instead 🙂 Maybe I should add get_worksheet_data here as well? 🤔
There was a problem hiding this comment.
Reviewable status: 4 of 9 files reviewed, 2 unresolved discussions (waiting on @dionizh and @razvan-pro)
aimmo-game/simulation/worksheet/init.py, line 2 at r1 (raw file):
For me I prefer __init__.py empty as much as possible, so it's one less file to check and modify 🙂. I saw that at the other places you import as:
from simulation.worksheet import WorksheetData from simulation.worksheet.worksheet import get_worksheet_data
So maybe we can keep it consistent and simple, so both import from simulation.worksheet.worksheet then empty the __init__.py as you already changed the convention a bit 😁? But it's not a biggie if you just wanna leave it.
There was a problem hiding this comment.
Reviewable status: 4 of 10 files reviewed, 1 unresolved discussion (waiting on @dionizh and @razvan-pro)
aimmo-game/simulation/worksheet/init.py, line 2 at r1 (raw file):
Previously, dionizh (Dioni Zhong) wrote…
For me I prefer
__init__.pyempty as much as possible, so it's one less file to check and modify 🙂. I saw that at the other places you import as:from simulation.worksheet import WorksheetData from simulation.worksheet.worksheet import get_worksheet_dataSo maybe we can keep it consistent and simple, so both import from simulation.worksheet.worksheet then empty the
__init__.pyas you already changed the convention a bit 😁? But it's not a biggie if you just wanna leave it.
Agreed 😁
There was a problem hiding this comment.
Reviewed 6 of 6 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
Codecov Report
@@ Coverage Diff @@
## development #1476 +/- ##
===============================================
- Coverage 63.01% 62.95% -0.06%
===============================================
Files 166 165 -1
Lines 3658 3663 +5
Branches 224 228 +4
===============================================
+ Hits 2305 2306 +1
Misses 1327 1327
- Partials 26 30 +4
|
Because of Agones changes, aimmo-game doesn't load the correct worksheet ID at the moment. This is fine for now as worksheets 1 and 2 have the same data, but it will be needed for the third one which will have different properties.
Note this won't work if the game worksheet is changed after the game is running - we will fix this at a later stage.
This change is