search

cutelyaware / magiccube4d

Automatically exported from code.google.com/p/magiccube4d
Other
71 stars 16 forks source link

Make colours from facecolors.txt be loaded upon puzzle creation #146

Closed KittyKittyKitKat closed 3 years ago

KittyKittyKitKat commented 3 years ago

This solves the issue of the facecolors.txt file not being detected and loaded in unless the puzzle is changed or a log file is loaded. (Referenced by Issues #95 and #129) This problem was caused by the code for loading in the colours, namely the findColors(schlafi, len) method, which was only called in the PuzzleListener of the PuzzleManager object created. This directly caused the issue described above. To remedy this, I've simply transferred this call to findColors, as well as some additional logic handling the result, to a more appropriate location. The location I chose was the initPuzzle method of the PuzzleManager class. I also adapted the logic slightly to fall back on the preexisting default call of generateVisuallyDistinctColors in the case the facecolors.txt file did not yield appropriate or correct colours. To facilitate this, I changed the scope of the findColors method from private to public,

KittyKittyKitKat commented 3 years ago

Forgive the accidental closure

roice3 commented 3 years ago

Looks good to me. We don't have a release pipeline, so this may not go out for a while.

cutelyaware commented 3 years ago

I appreciate the bug fix but I have a problem with the implementation. MC4DSwing is the highest level code, so lower level code such as PuzzleManager shouldn't know about it or make calls into it. What's the easiest way to reproduce the problem?

KittyKittyKitKat commented 3 years ago

For me, the problem was easily reproduced after a fresh download, and making a facecolors.txt file. Upon program start the facecolors.txt file is not recognized. I was able to force recognition by loading in a log file.

I understand your issue with the implementation, and to be perfectly honest I took the route of least resistance and didn't really consider the program architecture and hierarchy. I'm sure there's a method to resolve this while keeping the integrity of the code.

cutelyaware commented 3 years ago

Yeah, the fastest path often requires the most work by adding technical debt. I fixed this by moving the color file reading code from MC4DSwing into ColorUtils.

KittyKittyKitKat commented 3 years ago

Very sound decision on your part. I looked at the commit and it all looks great. Glad I could be of some help to this very cool project!

cutelyaware commented 3 years ago

I very much appreciate your effort!