Veloxization
commented
2 years ago Thank you for your valuable feedback!
Non-accessible areas were indeed intentional in the version you reviewed, though they are getting attached to other areas or pruned as dead ends in the current and future versions. :) Other visual hiccups you commented on were also fixed with this pruning, luckily.
Thank you for your feedback on the naming conventions and the structure of the map class. I've been trying to specify the possibly unclear parts in the docstrings but will revisit these at a later date if time allows!
Jiisala
Time of download. 17.2.2022.
Code review and some thoughts about the working of the program.
The general structure of the program is clean, well documented and easy to follow. Automated unit tests seem to test relevant things. I have no suggestions of improvement for them. The room generation and the image generation seem to work well already, I have no suggestions on the either.
The map entity is at parts bit confusing to read, especially the occupy function I’m pretty sure it could be refactored a bit to get rid of the many nesting if statements. However it seems to what it is supposed to do, so I doubt that refactoring it is really high on your priority list.
I spent most of my time with your program studying what I believe is the main part of the program, the maze generation class. It is also a bit hard to follow at times, even when I know how Prims’s algorithm works. The main confusion for me was the use of “p”, “r” and “w”. It might be easier to follow the code if something more informative, was used in their place.
I generated a bunch of maps with different dimensions and room configurations. There seems to be a bug somewhere (I suspect the Prim’s, but I could not pinpoint it) that creates closed non accessible areas in the maze. At least I think that it is not intended feature. Thinking of the purpose of your program, that might be used as an advantage. If you device a way of finding them (I suggest floodfill or equivalent technique) you could use them as secret rooms.
The thing that bothered me visually the most was the tendency of the maze to cut away corners of the rooms, but that should be quite easy to fix, and I suspect that it is something that you just have not gotten around yet. My personal taste would also prefer a maze that does not have diagonally adjacent corridors, but that is just a matter of preference. If you plan on replacing the square tiles with lines later, the diagonal walls will probably create a nice look for the maze.
If I may, I have a suggestion for an easy work around for your current problem, of connecting the rooms to corridors. You could maybe just choose a random spot on the wall of the room, make note of the orientation (eg. is it top, bottom, right, or left wall) and “dig” away from the room until you pop in to a open space. If you plan on keeping the look of your maze as it is it is just a matter of removing one or two tiles. The first spot should probably get marked as a door for future use. The probability of loops, could then maybe get used as a means of deciding if the rooms should generally have one or many doors.
All in all, good work this far.
A screenshot as an example of a closed area in a map: