SudokuMonster / SukakuExplainer

Modification to Sudoku Explainer (SE) by Nicolas Juillerat to accommodate Sukaku (Pencilmark) Sudoku puzzles
GNU Lesser General Public License v2.1
61 stars 13 forks source link

Master branch can't compile on java version prior 10 #13

Closed dobrichev closed 4 years ago

dobrichev commented 4 years ago

This branch by Sudokuwiki introduces new syntax var and the master branch (including release v1.2.1.5) cannot be compiled on java versions prior to 10. Can we fix this and try writing code compatible with older java 8 version which IMHO is still the mainstream version?

SudokuMonster commented 4 years ago

Hopefully this would allow also parsing vanilla sudoku grids similar to this format as well:

. . . | . . . | . . . 2 . . | . . . | . . 9 . . . | 7 5 4 | . . .

5 . 4 | . . . | 6 . 1 . 2 . | . . . | . 5 . . 3 1 | . . . | 4 9 .

. . 9 | 8 . 2 | 5 . . . 5 . | 4 . 6 | . 1 . . 6 . | 9 7 5 | . 2 .

dobrichev commented 4 years ago

OK, but this is shifting the target of the project and also enforces me to clone the repository and work only on my clone without intending to merge the changes back here. I'm definitely not going to change the java version of my work environment because of this project.

SudokuMonster commented 4 years ago

resolving the issue then branching GUI only will not interfere with the command line entry aspect of the project?

I'll wait for sudokuwiki to suggest a change that works. alternatively we can revert to version that was compatible and then branch the GUI separately. Pull it back only if eberybody is happy with the changes

dobrichev commented 4 years ago

Surely the same functionality can be rewritten in more conventional way.

SudokuMonster commented 4 years ago

I'll move ahead and revert. Sudokuwiki's changes are now in the the GUI branch which compiles except under he most recent JAVA release

1to9only commented 4 years ago

Also the 1.2.1.5 SukakuExplainer requires Java Runtime (class file version 56.0) to run JDK-12. I am running with JDK-8. To resolve download and install the latest JDK.

SudokuMonster commented 4 years ago

Unfortunately. I tried to revert to the Aug 5 release but ran into issues with SudokuIO.java, something that dobrichev addressed with a commit in the file history. The resulting fix however, changed how the GUI handles IO. I'm therefore painfully reverting to the latest release 1.2.1.5 again with the 3 files which @sudokuwiki modified appearing. Your earlier request to modify the code in these 3 files to a Java 8 compliant code still needs to be addressed

I also deleted the GUI branch as it wasn't needed anymore after reverting.

The 1.2.1.5 release as far as initial testing shows is stable and compiles well under the latest JAVA releases which I had to download as I didn't have any of them on this computer

You are correct in requesting the most stable and most compatible release to be in the Master branch because that allows you to work on the parallel chaining branch with the tools you have. I would also avoid forking if possible at this stage

dobrichev commented 4 years ago

What are the expected next steps?

SudokuMonster commented 4 years ago

Possible next steps:

  1. accept current state of JAVA compatibility: No action needed directly to Master branch. Your working tools on any branches should be compatible with that level of JAVA
  2. change the Master branch directly (3 files @sudokuwiki modified + any others) to make it compatible with Java 8. This person should ideally be @sudokuwiki but it can be anybody with code compatibility knowledge

if step 2 is the way forward … then the Master code needs to be tested to check it compiles with JAVA 8 and that it works (GUI not broken, ... etc). A new release v1.2.1.6 follows that & then work resumes on the branches. Reverting to to Aug 5 release for the 3 mentioned files caused a jar compile error. I decide to revert back to v.1.2.1.5 release & suggest the possible above steps instead.

dobrichev commented 4 years ago

Just for fun, I am reading the first option as

Don't accept current state of JAVA compatibility: No action needed from you directly to Master branch.

How frequently from now on is expected actions similar to step 2 to appear?

I am asking because I was almost ready to merge my work into the master but now I have to deal with 18 almost identical fixes of the new java reserved word "var", in code that implements functionality not discussed here, being not sure the fixes wouldn't be overwritten soon by a similar commit.

SudokuMonster commented 4 years ago

@sudokuwiki only updated these 3 files with a single commit. I'm not sure if he is aware of the compatibility issues that we are discussing. I'm not sure if he is going to do any more collaborating but he hasn't replied to some of the issues raised.

With @1to9only modifiying the code by proxy as he hasn't collaborated in the repository directly through commits, it will be either me or you who currently modify the code. I will not be modifying the Master JAVA files now unless after a branch pull request is discussed and agreed.

dobrichev commented 4 years ago

I've fixed Java 8 compatibility issue in SudokuIO.java. Also aligned branch parallelChaining to master.

Now I will open a merge request for parallelChaining to master making visible all my changes to master. After merging (and testing prior or after the merging) to master, I will close this issue.

Please, help in testing because I don't know what to test in GUI.

SudokuMonster commented 4 years ago

This is very exciting ... yes absolutely.

We should have a set of puzzles of known ER/EP/ED rating from the patterns game in addition to the known relatively easy Sukakus that we know their rating from 1to9only early modifications. I’ll handle the GUI testing because that is hands on!!

I can prepare the set of puzzles on a wiki page but it may take until midnight GMT to compile

Well done!!

sudokuwiki commented 4 years ago

Don't know why var is an issue. But if old java does not accept this, then you could write the full type. It compiles on my pc, strangely the IDE does not detect for which version it should compile. Sort of no real experience with git/guidelines.

sudokuwiki commented 4 years ago

Maybe there could be an extra parser for multiline grid @SudokuMonster mentioned.

sudokuwiki commented 4 years ago

Issue can be closed I guess, can someone verify?

dobrichev commented 4 years ago

I just verified that some committed on master this breaking the Pull Request. 8 hours ago I asked how frequently similar issues is expected to appear...