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

Minor code refactor for improved readability #4

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Arduino88
Copy link

This is a cool project, I really like the idea!
I haven't changed any core logic or gameplay, only improved the readability of code and swapped some if-else chains for match statements.
I also combined an unnecessary double-if check near the end.

adjusted movement logic for readability,
cleaned up unnecessary comments,
removed small redundancies
@pristinecodes
Copy link

Womp Womp

@VenroyDEV
Copy link

I am currently jelqing it to the skibidi map, 10/10, I skibidied all over the floor!

@odpay
Copy link
Owner

odpay commented May 23, 2024

@Arduino88 i appreciate the interest, sorry i didn't comment sooner.

  • i'd prefer to keep movement directions as dynamic tuples, the naming of p1.up(), p1.down(), etc. intrinsically reveals their direction in light of the messy system of moving a player across index positions in a nested list (grid[])
    • and i plan to implement future mechanics which allow for a speed greater than 1/tick, so i'll keep this as is.
  • match case statements are great (and more efficient i believe), they can just be finicky and have weird requirements to work ( see Use modern python functionalities #6 & FixedBruz #7 ) , if there are no issues then feel free to implement.
  • any other changes boil down to convention, removing comments, removing newlines. I know my code is messy, but it'll be easier for me if none of those things are changed until a first official release.

if you'd like to rebase from latest commit and update your pr with just the match case changes, i'd be happy to merge it.

perhaps have a go at designing a level, see the readme.

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

4 participants