Adding a map creation plugin #57

Merged
CoCo_Sol merged 39 commits from map-generation into main 2024-02-21 20:10:03 +00:00
2 changed files with 24 additions and 21 deletions
Showing only changes of commit 3cf5d76d00 - Show all commits

View file

@ -35,36 +35,45 @@ pub struct StartMapGeneration {
#[derive(Event)] #[derive(Event)]
pub struct EndMapGeneration; pub struct EndMapGeneration;
CoCo_Sol marked this conversation as resolved Outdated

You should follow the bevy convention. Events in bevy doen't finish with "Event".

You should follow the bevy convention. Events in bevy doen't finish with "Event".
/// Spawns the tiles if the event is received. /// Generate each tiles of the map if the [StartMapGeneration] is received.
///
/// The map is generated using the [Perlin] noise function and the [HexSpiral].
///
/// It's generated one tile at a time, until the spiral is finished.
fn generate_map( fn generate_map(
CoCo_Sol marked this conversation as resolved Outdated

Perlin isn't a function

Perlin isn't a function
mut event: EventReader<StartMapGeneration>, mut start_generation_events: EventReader<StartMapGeneration>,
CoCo_Sol marked this conversation as resolved Outdated

Isize will not be great to use for networking because it's length depend on the system that runs the program.
Also you should use type to alias a specific type for the HexPosition used for the map.

Isize will not be great to use for networking because it's length depend on the system that runs the program. Also you should use `type` to alias a specific type for the HexPosition used for the map.

I use only one time the hex position, is it really better to create a new type ?

I use only one time the hex position, is it really better to create a new type ?

You will use it every time you want to make a Query on the HexPosition

You will use it every time you want to make a Query on the HexPosition

ok, you've convinced me

ok, you've convinced me
mut end_map_event: EventWriter<EndMapGeneration>, mut end_generation_writer: EventWriter<EndMapGeneration>,
mut commands: Commands, mut commands: Commands,
mut local_noise: Local<Option<Perlin>>, mut local_noise: Local<Option<Perlin>>,
mut local_spiral: Local<Option<HexSpiral<i32>>>, mut local_spiral: Local<Option<HexSpiral<i32>>>,
) { ) {
// Handle map generation events. // Handle map generation events and create the spiral and the noise.
for event in event.read() { for event in start_generation_events.read() {
*local_noise = Some(Perlin::new(event.seed)); *local_noise = Some(Perlin::new(event.seed));
CoCo_Sol marked this conversation as resolved Outdated

You can use the let else syntax to remove one level of nesting.

You can use the let else syntax to remove one level of nesting.

if I use the "else let" keyword, the nesting level is the same

if I use the "else let" keyword, the nesting level is the same

No because you do an early return

No because you do an early return

I have to write an unwrap so ?

I have to write an unwrap so ?

No, an let else

No, an let else
*local_spiral = Some(TilePosition::new(0, 0).spiral(event.radius as usize)); *local_spiral = Some(TilePosition::new(0, 0).spiral(event.radius as usize));
} }
// Check if the map is being generated.
let (Some(noise), Some(spiral)) = (local_noise.as_ref(), local_spiral.as_mut()) else { let (Some(noise), Some(spiral)) = (local_noise.as_ref(), local_spiral.as_mut()) else {
return; return;
}; };
// Spawn a tile until the spiral is finished
// If the map is generated, we send [EndMapGeneration] and set the local
// variables to None.
if let Some(position) = spiral.next() { if let Some(position) = spiral.next() {
let pixel_position = position.to_pixel_coordinates((0.2, 0.2)); let pixel_position = position.to_pixel_coordinates() * 0.2;
CoCo_Sol marked this conversation as resolved Outdated

Why don't you use HexPosition here?

Why don't you use HexPosition here?

because is not a hex position in a hex grid but a position in orthogonal grid

because is not a hex position in a hex grid but a position in orthogonal grid

You need to change the name of the function or take an HexPosition and convert it in the function, because it can be confusing that you use "tile" to describe the orthogonal position of an HexPosition

You need to change the name of the function or take an HexPosition and convert it in the function, because it can be confusing that you use "tile" to describe the orthogonal position of an HexPosition

you are right

you are right
CoCo_Sol marked this conversation as resolved
Review

This should be in the get_type function and the value shouldn't be hard coded

This should be in the get_type function and the value shouldn't be hard coded
commands.spawn((get_type(pixel_position, noise), position as TilePosition)); commands.spawn((get_type(pixel_position, noise), position as TilePosition));
} else { } else {
end_map_event.send(EndMapGeneration); end_generation_writer.send(EndMapGeneration);
*local_noise = None; *local_noise = None;
*local_spiral = None; *local_spiral = None;
} }
} }
/// Returns the type of the position with the given noise. /// Returns the type of the position with the given noise.
CoCo_Sol marked this conversation as resolved Outdated

This comment is wrong.

This comment is wrong.
fn get_type(position: (f32, f32), noise: &Perlin) -> Tile { fn get_type(position: Vec2, noise: &Perlin) -> Tile {
CoCo_Sol marked this conversation as resolved Outdated

I still think you should take an HexPosition, this will be simpler and will make this function simpler to use.

I still think you should take an HexPosition, this will be simpler and will make this function simpler to use.
let value = noise.get([position.0 as f64, position.1 as f64]); let value = noise.get([position.x as f64, position.y as f64]);
match value { match value {
CoCo_Sol marked this conversation as resolved Outdated

get_tile_type

get_tile_type
v if v <= -0.4 => Tile::Hill, v if v <= -0.4 => Tile::Hill,
CoCo_Sol marked this conversation as resolved Outdated

This is not a Zoom, it is a Scale

This is not a Zoom, it is a Scale
v if v >= 0.4 => Tile::Forest, v if v >= 0.4 => Tile::Forest,

View file

@ -240,7 +240,6 @@ impl<T: Number> HexPosition<T> {
} }
/// Converts the current [HexPosition] into a pixel coordinate. /// Converts the current [HexPosition] into a pixel coordinate.
/// Input: The size of the hexagon in pixels (witdh, height).
/// ///
/// If you want to learn more about pixel coordinates conversion, /// If you want to learn more about pixel coordinates conversion,
/// you can check the /// you can check the
@ -252,18 +251,13 @@ impl<T: Number> HexPosition<T> {
/// use border_wars::map::hex::HexPosition; /// use border_wars::map::hex::HexPosition;
/// ///
/// let position = HexPosition(1, 0); /// let position = HexPosition(1, 0);
/// assert_eq!( /// assert_eq!(position.to_pixel_coordinates(), (3f32.sqrt(), 0.0));
/// position.to_pixel_coordinates((1.0, 1.0)),
/// (3f32.sqrt(), 0.0)
/// );
/// ``` /// ```
pub fn to_pixel_coordinates(&self, size: (f32, f32)) -> (f32, f32) { pub fn to_pixel_coordinates(&self) -> Vec2 {
( Vec2::new(
size.0 3f32.sqrt()
* 3f32
.sqrt()
.mul_add(T::to_f32(self.0), 3f32.sqrt() / 2.0 * T::to_f32(self.1)), .mul_add(T::to_f32(self.0), 3f32.sqrt() / 2.0 * T::to_f32(self.1)),
size.1 * (3.0 / 2.0 * T::to_f32(self.1)), 3.0 / 2.0 * T::to_f32(self.1),
) )
} }