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 basic tetris to dev #69

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Conversation

petabyt
Copy link

@petabyt petabyt commented Sep 1, 2022

A few notes:

  • "ptetris" is a minimal tetris library I made. It interfaces with a common "binding" to IO. https://github.com/petabyt/ptetris/
  • Code doesn't recompile when ptetris.h is modified
  • Needs better field checking. Currently doesn't check out of field after rotating
  • menu_redraw_blocked is set to 1 to prevent tetris rendering to flicker as menu is being drawn.
  • task_create is called to keep the ML menus in place after the game is quit.

@@ -0,0 +1,7 @@
# Force format
$(info $(shell astyle --style=allman tetris.c ptetris.h))
Copy link
Owner

Choose a reason for hiding this comment

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

This looks like it adds a dependency to the ML build system for "astyle", since all modules are built for all cams. I don't really want to do that. If you're volunteering to convert all ML code to be compatible, and then add an astyle dep, that's different! But it also feels like a separate task.

Probably remove this dep?

Copy link
Author

Choose a reason for hiding this comment

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

I can comment it out.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks. Annoyingly, there's an "official" coding style in doc/CODING_STYLE - which even suggests using astyle! But it is very clear this is not followed consistently.

Copy link
Author

@petabyt petabyt Oct 28, 2022

Choose a reason for hiding this comment

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

Would be nice if the entire repo was formatted consistently, but that would be too big of a diff and mess with history+branches


// Max array size
#ifndef PT_MAX_WIDTH
#define PT_MAX_WIDTH 40
Copy link
Owner

Choose a reason for hiding this comment

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

This file uses tabs, the .c uses 4-spaces. 4-spaces is preferred, please convert this file.

Copy link
Author

Choose a reason for hiding this comment

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

This is a copied directly from my ptetris project, which uses tabs. So I would have to convert the file to spaces every time I update the file. Not a big deal.

@reticulatedpines
Copy link
Owner

Test merge worked. Bugs found:

  • pieces can spawn outside of the boundary, and then they cannot be moved into play. Seems to happen if piece was dropped at either edge and cursor left there, some pieces then spawn incorrectly
  • square block never spawns, so presumably something around the RNG is incorrect
  • sleeploop variable is never referenced and should be removed (triggers a compiler warning)
  • the left- and right-handed S pieces are the same colour! It hurts my brain!

It's a cool mini-tetris :)

@petabyt
Copy link
Author

petabyt commented Oct 28, 2022

Some of those issues seem familiar. I've got an updated ptetris.h that I never pushed, I'll add it later

No idea what I did, unformatted, sorry
- Less screen tearing, but perfect, but playable
- Fixed RNG
- Fixed block colors
- Increased size of play area
@petabyt
Copy link
Author

petabyt commented Nov 25, 2023

Fixed all of the things you mentioned. IMO it's very playable at this point. Could still use some minor improvement (better visuals, better controls) but for tetris on a DSLR it's not bad :)

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.

2 participants