-
Notifications
You must be signed in to change notification settings - Fork 6
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
Info Display enhancement : Rotation, Rotation Speed, TimeDifference, Oriented Speed. #57
base: main
Are you sure you want to change the base?
Conversation
Added rotation and rotation speed to the infodisplay. Should be ready to use.
Fixed an issue in race manager : RaceState is 4 when the race is finished, not 3. (atleast for TTs). It was previously causing the info display to turn off on the race finish state, despite the code not aiming to do that. Fixed the Rotation speed calculation to not trigger error while not in a race. (It wasn't a big issue, it was just spamming the python log)
Added time difference to info display Changed some code for the rotation speed. Added a class to store data from previous frames. Added Oriented speed to infodisplay
Small changes to TimeDiff, to make the code more clean, and do less computations. Added Advanced Ghost Comp. (might still change a bit) Added a Bruteforcer (still wip)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Writing a pre-test review to make sure these comments are addressed prior to merge.
.gitignore
Outdated
scripts/Modules/test_del_me_later.py | ||
scripts/Modules/test.bit | ||
scripts/Modules/test_del_me_later.py | ||
scripts/Modules/test_del_me_later.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be deleted prior to merge.
scripts/Modules/infodisplay.ini
Outdated
@@ -0,0 +1,39 @@ | |||
[DEBUG] | |||
debug = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entire file should be taken out of this PR prior to merge to avoid conflict.
# TODO: Time difference display helper functions | ||
"""The time difference functions. | ||
time_difference_[name](P1, S1, P2, S2) is a function that takes as arguments | ||
P1,S1 : Player1's Position and Speed vec3. | ||
P2,S2 : Player2's Position and Speed vec3 | ||
Return the time it would take for Player1 to catch Player2 (not always symmetric) | ||
|
||
get_time_difference_[name](Player1, Player2) takes as arguments | ||
Player1 : Player1 ID | ||
Player2 : Player2 ID | ||
Return the time it would take for Player1 to catch Player2 (not always symmetric) | ||
It's the function called in draw_infodisplay.py | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this WIP?
Dolphin will do this already, no need to clutter the PR with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes should appease the linter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the purpose of this is to have a placeholder file to keep the folder, .gitkeep
is the preferred name. Otherwise, messages or notes are best done in .txt
or .md
file bodies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the only purpose of that text file, delete it if it's not needed
@@ -1 +1,2 @@ | |||
/scripts/MKW_Inputs/*.csv | |||
scripts/AGC_Data/ghost.data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consistency.
scripts/AGC_Data/ghost.data | |
/scripts/AGC_Data/ghost.data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid spaces in filenames. TimeDifferenceInfo.txt
would be my pick.
Optionally, this can be improved via markdown.
def float_to_str(f): | ||
ms = round((f % 1)*1000) | ||
s = math.floor(f) % 60 | ||
m = math.floor(f)//60 | ||
return f"{m},{s},{ms}" | ||
|
||
|
||
def floats_to_str(fs): | ||
return f"{float_to_str(fs[0])};{float_to_str(fs[1])};{float_to_str(fs[2])}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Old functions that i forgot to remove when I changed the implementation of TimerData. You can delete
return [(a+0x68, 12), # Position | ||
(a+0xF0, 16), # Rotation | ||
(a+0x74, 12), # EV | ||
(a+0x14C, 12), # IV | ||
(b+0x18, 4), # MaxEngineSpd | ||
(b+0x20, 4), # EngineSpd | ||
(b+0x9C, 4), # OutsideDriftAngle | ||
(b+0x5C, 12)] # Dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that these are blindly referenced (e.g. addr[0]
, addr[1]
, etc). There are two alternatives to solve this:
- Return a dict with key-value pairs, where the keys are these comments
- Create a dataclass with a static
read
method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know about static read method was a thing when i wrote that, that's why some init functions are a mess.
This class will probably be rewritten and merged with mkw_utils.FrameData at some point
def __mul__(self, other): | ||
""" vec3 * vec3 -> float (dot product) | ||
vec3 * float -> vec3 (scalar multiplication)""" | ||
if type(other) == vec3: | ||
return self.x * other.x + self.y * other.y + self.z * other.z | ||
else: | ||
return vec3(self.x * other, self.y * other, self.z * other) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of the ambiguous type return. It would likely be better to have a designated dot
method.
if type(other) == vec3:
return vec3(self.x * other.x, self.y * other.y, self.z * other.z)
else:
return vec3(self.x * other, self.y * other, self.z * other)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 3 types of vector multiplications useful in different context.
The dot product, which takes two vec3 and return a number (float typically)
The cross product, which takes two vec3 and return a vec3
The scalar multiplication, which takes a vec3 and a number, and return a vec3
But I knew only 2 multiplicatives token for python class (being "" and "@")
So I used "" for the dot product and the scalar multiplication, and "@" for the cross product and scalar multiplication.
If the ambiguous return type is really a problem, i'd rather have "*" for dot product, "@" for cross product, and a "scalarmult" method for the scalar multiplication.
But I don't mind this ambiguity personally, as it allow for some code very simple to read imo.
def __matmul__(self, other): | ||
""" vec3 @ vec3 -> vec3 (cross product) | ||
vec3 @ float -> vec3 (scalar multiplication)""" | ||
if type(other) == vec3: | ||
x = self.y*other.z - self.z*other.y | ||
y = self.z*other.x - self.x*other.z | ||
z = self.x*other.y - self.y*other.x | ||
return vec3(x,y,z) | ||
else: | ||
return vec3(self.x * other, self.y * other, self.z * other) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, a designated cross
method would also likely be better.
Out of curiosity, what is __matmul__
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type ambiguity is less of a problem here since matmul always return a vec3 in this context
matmul is a class method, just like add or mul. It uses the token "@".
So you write a@b and it's interpreted as a.matmul(b)
Here it design the cross product
@staticmethod | ||
def from_bytes(bts) -> "vec3": | ||
return vec3(*struct.unpack('>' + 'f'*3, bts)) | ||
|
||
def to_bytes(self) -> bytearray: | ||
return bytearray(struct.pack('>fff', self.x, self.y, self.z)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional:
@staticmethod | |
def from_bytes(bts) -> "vec3": | |
return vec3(*struct.unpack('>' + 'f'*3, bts)) | |
def to_bytes(self) -> bytearray: | |
return bytearray(struct.pack('>fff', self.x, self.y, self.z)) | |
@staticmethod | |
def unpack(bts) -> "vec3": | |
return vec3(*struct.unpack('>' + 'f'*3, bts)) | |
def pack(self) -> bytearray: | |
return bytearray(struct.pack('>' + 'f'*3, self.x, self.y, self.z)) |
|
||
# These are helper functions that don't quite fit in common.py | ||
# This file also contains getter functions for a few global variables. | ||
|
||
# NOTE (xi): wait for get_game_id() to be put in dolphin.memory before clearing | ||
# these commented-out lines: | ||
|
||
|
||
class FrameData: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this potentially conflict with agc_lib.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can conflict if you import both class with the same name.
Currently it's not an issue, but if soemone makes some script using both agc_lib and mkw_utils, it could be.
I'll eventually merge both class into the same class (currently, agc_lib.FrameData store values you can read in the memory, and mkw_utils.FrameData is more messy, but can store function from values you can read in the memory, and only used for the draw_info_display.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename the folder to exclude non-alphanumeric characters (e.g. AdvGhostComp
).
First attempt at implementing TTK with 7 columns
Oriented Speed is the XZ speed, but instead of decomposing it as X and Z, it's decomposed as Forward and Sideway (relative to your facing angle)
This PR also contain the advanced ghost comp, but the implementation might change in the future.