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 Startslide Scripts #30

Merged
merged 2 commits into from
Sep 27, 2023
Merged

Add Startslide Scripts #30

merged 2 commits into from
Sep 27, 2023

Conversation

ximk
Copy link
Member

@ximk ximk commented Sep 24, 2023

Adds startslide functionality using .csv files.

With linter errors fixed and updated startslides.
scripts/Startslide_Left.py Show resolved Hide resolved
scripts/Startslide_Left.py Outdated Show resolved Hide resolved
scripts/Startslide_Right.py Outdated Show resolved Hide resolved
scripts/Startslide_Right.py Outdated Show resolved Hide resolved
scripts/Startslide_Left.py Outdated Show resolved Hide resolved
scripts/Startslide_Right.py Outdated Show resolved Hide resolved
scripts/Startslide_Left.py Outdated Show resolved Hide resolved
scripts/Startslide_Right.py Outdated Show resolved Hide resolved
@ximk ximk force-pushed the startslide_v2 branch 3 times, most recently from 98d0729 to 4057e1c Compare September 27, 2023 02:45
@ximk ximk requested a review from malleoz September 27, 2023 03:12
Comment on lines 42 to 51
if vehicle in flame_slide_bikes:
path = os.path.join(path, r"MKW_Inputs", r"Startslides", r"flame_left.csv")

elif vehicle in spear_slide_bikes:
path = os.path.join(path, r"MKW_Inputs", r"Startslides", r"spear_left.csv")

elif vehicle in star_slide_bikes:
path = os.path.join(path, r"MKW_Inputs", r"Startslides", r"star_left.csv")

return path
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of updating path in each of these if-statements, you could just return os.path.join(path ...)

path = utils.get_script_dir()

if vehicle in flame_slide_bikes:
path = os.path.join(path, r"MKW_Inputs", r"Startslides", r"flame_left.csv")
Copy link
Collaborator

@malleoz malleoz Sep 27, 2023

Choose a reason for hiding this comment

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

Are these raw strings necessary anymore, since you got rid of the slashes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unsure, this was just what @Gaberboo recommended to me.


flame_slide_bikes = ("Flame Runner", "Mach Bike", "Sugarscoot", "Zip Zip")
spear_slide_bikes = ("Jet Bubble", "Phantom", "Spear", "Sneakster", "Wario Bike")
star_slide_bikes = ("Bit Bike", "Bullet Bike", "Dolphin Dasher", "Magikruiser",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was just curious, is there any performance different using a tuple here instead of a list?

Copy link
Member Author

Choose a reason for hiding this comment

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

I doubt the performance is significantly different whether a tuple was used or not, I thought I might as well use one here in case it did.

Comment on lines 43 to 52
if vehicle in flame_slide_bikes:
path = os.path.join(path, r"MKW_Inputs", r"Startslides", r"flame_right.csv")

elif vehicle in spear_slide_bikes:
path = os.path.join(path, r"MKW_Inputs", r"Startslides", r"spear_right.csv")

elif vehicle in star_slide_bikes:
path = os.path.join(path, r"MKW_Inputs", r"Startslides", r"star_right.csv")

return path
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto

- Fixed module import with integration of TASLabz/dolphin@61f6735

- Removed unnecessary memory checks

- Updated TTK call to reflect new version

- Removed unnecessary `global playerInputs` call

- Fixed linter errors

- Use `os.path.join()` to retrieve path

- More linter error fixes

- Removed path separators

- Final fixes after review
@malleoz malleoz merged commit 2772dc5 into TASLabz:main Sep 27, 2023
1 check passed
@Gaberboo Gaberboo mentioned this pull request Oct 10, 2023
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