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 maze order (1 or 0) corresponding to the maze density #101

Merged
merged 4 commits into from
Sep 24, 2024

Conversation

bal7hazar
Copy link
Collaborator

Introduced changes

Add a new argument for maze and corridor generations, the density order.
The order of the maze, it must be 0 or 1, the higher the order the less dense the maze will be.

Note: I didnt managed higher orders since the definition is ambiguous and the scaling could be a problem.

Here an example:

Order 0 (current implementation)

0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 1 0 1 1 1 0 1 1 1 1 1 1 1 1 0 1 0
0 1 1 0 1 0 1 1 0 0 1 0 0 0 1 1 1 0
0 0 1 1 1 1 0 1 1 0 1 1 1 1 0 0 1 0
0 1 1 0 0 1 0 0 1 1 0 1 0 1 0 1 0 0
0 1 0 0 0 1 1 1 1 0 1 0 0 1 1 1 1 0
0 1 1 0 1 0 0 0 0 0 1 1 1 0 1 0 1 0
0 0 1 1 1 0 1 1 1 1 0 1 0 1 0 1 1 0
0 1 0 1 0 1 1 0 0 1 1 1 0 1 1 1 0 0
0 1 0 1 1 1 0 1 1 1 0 1 1 0 1 0 1 0
0 1 1 0 0 0 1 0 1 0 1 1 0 1 1 1 1 0
0 0 1 1 1 1 1 1 0 1 1 0 0 1 0 1 0 0
0 1 1 0 1 0 0 1 1 1 0 1 1 1 0 1 1 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

Order 1 (new implementation, with the same seed)

0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 1 1 1 1 1 0 0 1 1 1 1 1 1 1 1 0 0
0 0 1 0 0 1 0 1 1 0 1 0 0 0 0 1 1 0
0 1 1 0 0 0 0 1 0 0 1 1 1 1 0 0 1 0
0 0 1 1 1 1 0 1 1 0 0 0 0 1 0 0 0 0
0 1 1 0 0 1 0 0 1 0 1 1 0 1 1 1 1 0
0 1 0 0 1 1 1 1 1 1 1 0 0 0 0 0 1 0
0 1 1 0 0 0 0 0 0 0 0 0 1 0 0 1 1 0
0 0 1 1 0 0 1 1 1 1 0 1 1 0 1 1 0 0
0 0 0 1 0 1 1 0 0 1 1 1 0 0 1 0 0 0
0 1 0 1 1 1 0 0 0 1 0 1 0 1 1 1 1 0
0 1 0 0 0 0 0 1 0 1 0 0 0 1 0 0 1 0
0 1 1 1 1 1 1 1 1 1 1 0 1 1 0 1 1 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

Basically, order 1 disallow these patterns (having 2 floors connected diagonally):

1 1 0
0 0 1
0 1 1

@bal7hazar bal7hazar requested a review from glihm September 17, 2024 15:16
Copy link
Contributor

@glihm glihm left a comment

Choose a reason for hiding this comment

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

The order is a bit abstract and maybe having an enum for supported values instead of a u8 is we can't support more than 2 values could be good.

Or if you feel like it will be possible to extend the order to any u8 then makes sense to keep it like so. 👍

let (x, y) = (position % width, position / width);
match direction {
Direction::North => (y <= height - 3)
Direction::North => (y < height - 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

the -2 here looks a bit magic, perhaps having this in a constant that have a more descriptive name could help the understanding.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same for some offset used in this function.

Copy link
Collaborator Author

@bal7hazar bal7hazar Sep 18, 2024

Choose a reason for hiding this comment

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

Actually 3 was weird but size - 2 is actually the range of the indexes (from 0 to size - 1) substracting 1 to ensure we didnt reached the edge yet.

It could be rewrite such as:

let last_index = height - 1;
let one_before_last_index = last_index - 1;
match direction {
    Direction::North => (y < one_before_last_index)
...
}

But maybe I could just add some comments to explain it, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment would be fine yeah, was trying to grasp the idea of those indexes. Thanks!

@bal7hazar bal7hazar merged commit 60ba6b4 into main Sep 24, 2024
10 checks passed
@bal7hazar bal7hazar deleted the feat/maze-order branch September 24, 2024 21:12
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