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

Puzzle Cell classes extend from GridCell as an integer #139

Closed
11 tasks done
Corppet opened this issue Jun 14, 2022 · 14 comments · Fixed by #142 or #443
Closed
11 tasks done

Puzzle Cell classes extend from GridCell as an integer #139

Corppet opened this issue Jun 14, 2022 · 14 comments · Fixed by #142 or #443
Assignees
Labels
enhancement Improvement to existing feature good first issue Good for newcomers priority This issue should be focused on first

Comments

@Corppet
Copy link
Collaborator

Corppet commented Jun 14, 2022

All puzzles currently have their cell class extend from GridCell as Integer types, yet they have their own respective types/enums. It may be a good idea down the line to change each cell class to use their own respective classes instead, which also means changing GridCell and PuzzleElement to work with enums.

For Example:

Current:

public class TreeTentCell extends GridCell<Integer>

Suggestion:

public class TreeTentCell extends GridCell<TreeTentType>

To do:

  • Revise the GridCell and PuzzleElement classes to work with enums. If the codebase seems more suitable to use Integer instead of their respective types, please comment about it below and provide some short reasoning if you can. Alternatively, you can store the integer data value within the enum class instead of GridCell and have your code perform arithmetic on that instead.

Change the cast types for each cell class...

  • Battleship
  • Fillapix
  • Heyawake
  • Lightup
  • Masyu
  • Nurikabe
  • Short Truth Table
  • Skyscrapers
  • Sudoku
  • Tree Tent
@Corppet Corppet added the enhancement Improvement to existing feature label Jun 14, 2022
@Corppet Corppet self-assigned this Jun 14, 2022
@Corppet Corppet added the good first issue Good for newcomers label Jun 14, 2022
@Corppet Corppet assigned Corppet and unassigned Corppet Jun 14, 2022
@Corppet Corppet linked a pull request Jun 14, 2022 that will close this issue
@Corppet Corppet removed their assignment Jun 17, 2022
@charlestian23 charlestian23 pinned this issue Aug 2, 2022
@charlestian23
Copy link
Collaborator

Note that if any changes are made here, we should double check that this tutorial is still up to date: https://github.com/Bram-Hub/Legup/wiki/Implementing-Puzzles

@charlestian23 charlestian23 added the priority This issue should be focused on first label Aug 11, 2022
@25tallurich 25tallurich self-assigned this Feb 3, 2023
@Acewvrs Acewvrs self-assigned this Feb 3, 2023
@pitbull51067 pitbull51067 self-assigned this Feb 3, 2023
@25tallurich
Copy link
Collaborator

Im doing LightUp rn

@ErinCohenGH ErinCohenGH self-assigned this Feb 3, 2023
@ErinCohenGH
Copy link
Collaborator

I can do Nurikabe

@Acewvrs
Copy link
Collaborator

Acewvrs commented Feb 3, 2023

For Sudoku, you'd actually want to use integer types. I'll now do SkyScrapers.

-- update: finished Skyscrapers and waiting for the PR review

@pitbull51067
Copy link
Collaborator

I'll do Fillapix

@pitbull51067 pitbull51067 reopened this Feb 3, 2023
@RyancRiv RyancRiv self-assigned this Feb 7, 2023
@RyancRiv
Copy link
Collaborator

RyancRiv commented Feb 7, 2023

I can do Skyscrapers

@ErinCohenGH
Copy link
Collaborator

I believe someone is already doing both of those

@Acewvrs
Copy link
Collaborator

Acewvrs commented Feb 10, 2023

heyawake doesn't have its custom types, so I think using integers is fine here too. I'll do masyu now

-- update: finished masyu. updated to the PR

@pitbull51067
Copy link
Collaborator

I'll do Tree Tent

Corppet added a commit that referenced this issue Feb 24, 2023
* Changes to `TreeTent`.

* Changes to `TreeTent`.

---------

Co-authored-by: Ivan Ho <[email protected]>
@Corppet
Copy link
Collaborator Author

Corppet commented Mar 17, 2023

I have already talked with some of you, but if you think the puzzle you are working on is using arithmetic with the data property in their respective GridCell and you think the generic type of the cell is better left as an Integer, please indicate so in a comment and provide some simple reason as to why.

@ErinCohenGH
Copy link
Collaborator

Nurikabe is also better left as an Integer because setting the type of cell is reliant on being able to set a numerical value in addition to just black or white.

@Corppet Corppet linked a pull request Mar 24, 2023 that will close this issue
10 tasks
@ErinCohenGH
Copy link
Collaborator

Is LightUp still being worked on?

@25tallurich
Copy link
Collaborator

LightUp has the same issue as heyawake and Sudoku the code does math on the numbers so it is not viable to do.

@25tallurich 25tallurich removed their assignment Apr 7, 2023
@ErinCohenGH
Copy link
Collaborator

I'll do Short Truth Table then, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to existing feature good first issue Good for newcomers priority This issue should be focused on first
Projects
None yet
7 participants