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

feat: add doors during level generation #41

Merged
merged 13 commits into from
Apr 29, 2024
Merged

Conversation

LukaBerkers
Copy link
Contributor

Description

This PR adds automatic placement of doors during level generation. The doors are placed when a room is placed, based on the available directions of that room. Each door gets a unique ID which we will use in the future to match doors with keys.

Testability

There is no level generation yet, but there is a hard-coded level in the TempFillFunction in GridPlacer.cs.

You can play this level in the GridSystem scene. The rooms should have the correct doors placed according to the hard-coded values in TempFillFunction. The doors should open when approached by the player.

Checklist

  • Set the proper pull request name
  • Merged main into your branch
  • Added a scene to the game for your changes

@LukaBerkers LukaBerkers changed the title Feat/68 spawn doors feat: add doors during level generation Apr 20, 2024
@LukaBerkers LukaBerkers force-pushed the feat/68-spawn-doors branch from 7233dd6 to 3a057e7 Compare April 22, 2024 16:50
@LukaBerkers LukaBerkers force-pushed the feat/68-spawn-doors branch from 3a057e7 to 0fda83c Compare April 24, 2024 10:40
@LukaBerkers LukaBerkers marked this pull request as ready for review April 24, 2024 11:55
@LukaBerkers LukaBerkers requested review from JensSteenmetz, Tboefijn, SilasPeters and wingbladedota and removed request for Tboefijn April 24, 2024 11:56
wingbladedota
wingbladedota previously approved these changes Apr 24, 2024
Copy link
Collaborator

@wingbladedota wingbladedota left a comment

Choose a reason for hiding this comment

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

LGTM :)

Copy link
Contributor

@SilasPeters SilasPeters left a comment

Choose a reason for hiding this comment

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

There is no test scene :(

aplib.net-demo/.editorconfig Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/WFC/GridPlacer.cs Outdated Show resolved Hide resolved
aplib.net-demo/Packages/manifest.json Show resolved Hide resolved
@LukaBerkers
Copy link
Contributor Author

LukaBerkers commented Apr 24, 2024

There is no test scene :(

The test scene is GridSystem. At the time I did not yet know that I should create my own new scene.

@SilasPeters
Copy link
Contributor

I don't thinkg perf: is a valid prefix for a commit, but I get where you are coming from.

Copy link
Contributor

@SilasPeters SilasPeters left a comment

Choose a reason for hiding this comment

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

Currently, doors are placed at every room for every direction they allow the player to go, period. This is not very flexible. How do you envision that your code will be used later? I expect that not every room needs to have doors from the beginning.

@JensSteenmetz
Copy link
Member

JensSteenmetz commented Apr 25, 2024

Old (closed) PR: #39
Reason for new PR: branch name was changed.

@LukaBerkers
Copy link
Contributor Author

I don't thinkg perf: is a valid prefix for a commit, but I get where you are coming from.

It is actually, see https://www.conventionalcommits.org/en/v1.0.0/#summary.

@LukaBerkers LukaBerkers requested a review from SilasPeters April 26, 2024 15:11
@LukaBerkers LukaBerkers requested review from JensSteenmetz and wingbladedota and removed request for JensSteenmetz and wingbladedota April 26, 2024 15:46
@LukaBerkers LukaBerkers enabled auto-merge (squash) April 26, 2024 15:48
@@ -193,6 +193,7 @@ private void PlaceDoors(int x, int z, Room room)
{
// # Calculate where the door should be placed

// North is in the negative x direction
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that that is how you coded the door placement, but why? I oriented all the prefabs and tile to have North be positive X.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but the grid is generated with north in the negative x direction.

@LukaBerkers LukaBerkers requested a review from SilasPeters April 28, 2024 09:18
Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

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

Looks great! Some small nitpicks.

{
PlaceTile(x, z, _grid[x, z].Tile);
}
for (int x = 0; x < _grid.Width; x++) PlaceTile(x, z, _grid[x, z].Tile);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: could also remove the brackets of the other for loop.

private void PlaceDoors(int x, int z, Room room)
{
Vector3 roomPosition = new(x * _tileSizeX, 0, z * _tileSizeZ);
// Get (half of) the depth of the door model
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
// Get (half of) the depth of the door model
// Get (half of) the depth of the door model.

Vector3 roomPosition = new(x * _tileSizeX, 0, z * _tileSizeZ);
// Get (half of) the depth of the door model
float doorDepthExtend = _doorRenderer.bounds.extents.z;
// Calculate the distance from the room center to where a door should be placed
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
// Calculate the distance from the room center to where a door should be placed
// Calculate the distance from the room center to where a door should be placed.

Comment on lines +204 to +206
// # Calculate where the door should be placed

// North is in the negative x direction
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
// # Calculate where the door should be placed
// North is in the negative x direction
// # Calculate where the door should be placed.
// North is in the negative x direction.

Comment on lines +196 to +197
float doorDepthExtend = _doorRenderer.bounds.extents.z;
// Calculate the distance from the room center to where a door should be placed
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
float doorDepthExtend = _doorRenderer.bounds.extents.z;
// Calculate the distance from the room center to where a door should be placed
float doorDepthExtend = _doorRenderer.bounds.extents.z;
// Calculate the distance from the room center to where a door should be placed

Comment on lines +194 to +195
Vector3 roomPosition = new(x * _tileSizeX, 0, z * _tileSizeZ);
// Get (half of) the depth of the door model
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
Vector3 roomPosition = new(x * _tileSizeX, 0, z * _tileSizeZ);
// Get (half of) the depth of the door model
Vector3 roomPosition = new(x * _tileSizeX, 0, z * _tileSizeZ);
// Get (half of) the depth of the door model

Comment on lines +217 to +220
// # Calculate the rotation the door should have

// The `RotateLeft` here is because the rotation of the grid and the rotation of the door model do not
// line up
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
// # Calculate the rotation the door should have
// The `RotateLeft` here is because the rotation of the grid and the rotation of the door model do not
// line up
// # Calculate the rotation the door should have.
// The `RotateLeft` here is because the rotation of the grid and the rotation of the door model do not
// line up.

Quaternion relativeDoorRotation = Quaternion.Euler(0, direction.RotateLeft().RotationDegrees(), 0);
Quaternion doorRotation = roomRotation * relativeDoorRotation;

// # Spawn the door
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
// # Spawn the door
// # Spawn the door.

Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for the markdown header syntax? In other comments as well?

Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

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

There might be a problem with the door placement, see Discord thread "fix: change boundary conditions for wave function collapse"

Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

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

The problem I mentioned earlier is not with the door placement.

@LukaBerkers LukaBerkers merged commit c0fb998 into main Apr 29, 2024
2 checks passed
@LukaBerkers LukaBerkers deleted the feat/68-spawn-doors branch April 29, 2024 08:30
Copy link

🎉 This PR is included in version 3.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants