Conversation
…ule-stabilization Feature/multi module stabilization
…ule-stabilization chore: remove obsolete release.yml and update Cargo.lock
There was a problem hiding this comment.
Pull request overview
This PR adds a nutrient system to the 3D voxel-based biological simulation and implements population merging functionality. It bumps the version from 0.1.0 to 0.2.0 and adds GitHub Actions workflows for automated releases.
Key changes:
- Added a
nutrientsfield to theVoxelstruct, enabling nutrient-based carrying capacity limits for populations - Implemented population merging logic to consolidate populations at the same location
- Added GitHub Actions workflows for stable and beta release automation
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/world3d.rs | Added nutrients field to Voxel struct and updated all constructor methods with default nutrient values |
| src/biology.rs | Implemented population merging, nutrient consumption, carrying capacity limits, and updated validation logic; translated comments to French |
| Cargo.toml | Version bump from 0.1.0 to 0.2.0 |
| Cargo.lock | Updated lockfile to reflect new version |
| CHANGELOG.md | Added changelog documenting version 0.2.0 changes |
| .github/workflows/release-stable.yml | Added workflow for automated stable releases from main branch |
| .github/workflows/release-beta.yml | Added workflow for automated beta releases from dev branch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uses: actions/create-release@v1 | ||
| with: | ||
| tag_name: ${{ steps.beta.outputs.new_beta_tag }} | ||
| release_name: Beta ${{ steps.beta.outputs.new_beta_tag }} | ||
| body: | | ||
| Beta build from dev. | ||
| draft: false | ||
| prerelease: true | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} No newline at end of file |
There was a problem hiding this comment.
The actions/create-release@v1 action is deprecated and will stop working soon. GitHub recommends using softprops/action-gh-release or the official GitHub CLI instead. See: https://github.blog/changelog/2024-02-29-deprecation-notice-v1-and-v2-of-the-artifact-actions/
| uses: actions/create-release@v1 | |
| with: | |
| tag_name: ${{ steps.beta.outputs.new_beta_tag }} | |
| release_name: Beta ${{ steps.beta.outputs.new_beta_tag }} | |
| body: | | |
| Beta build from dev. | |
| draft: false | |
| prerelease: true | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| uses: softprops/action-gh-release@v1 | |
| with: | |
| tag_name: ${{ steps.beta.outputs.new_beta_tag }} | |
| name: Beta ${{ steps.beta.outputs.new_beta_tag }} | |
| body: | | |
| Beta build from dev. | |
| draft: false | |
| prerelease: true |
| // Ajouter les nouvelles populations au map | ||
| for pop in new_populations.iter() { | ||
| let key = (pop.x, pop.y, pop.z, pop.species_id); | ||
| population_map | ||
| .entry(key) | ||
| .and_modify(|existing_size| *existing_size += pop.size) | ||
| .or_insert(pop.size); | ||
| } |
There was a problem hiding this comment.
The population merging logic is executed before retain_mut, which means new_populations is always empty at this point (line 53). This causes lines 67-74 to have no effect since they iterate over an empty vector. The merging should happen after new populations are generated inside retain_mut, not before.
| // Fusionner les populations proches sur le même voxel | ||
| let mut population_map = std::collections::HashMap::new(); | ||
|
|
||
| // Ajouter les populations existantes au map | ||
| for pop in populations.iter() { | ||
| let key = (pop.x, pop.y, pop.z, pop.species_id); | ||
| population_map | ||
| .entry(key) | ||
| .and_modify(|existing_size| *existing_size += pop.size) | ||
| .or_insert(pop.size); | ||
| } | ||
|
|
||
| // Ajouter les nouvelles populations au map | ||
| for pop in new_populations.iter() { | ||
| let key = (pop.x, pop.y, pop.z, pop.species_id); | ||
| population_map | ||
| .entry(key) | ||
| .and_modify(|existing_size| *existing_size += pop.size) | ||
| .or_insert(pop.size); | ||
| } | ||
|
|
||
| // Reconstruire la liste des populations | ||
| populations.clear(); | ||
| for ((x, y, z, species_id), size) in population_map { | ||
| populations.push(Population::new(species_id, x, y, z, size)); | ||
| } |
There was a problem hiding this comment.
The population merging happens before any populations are processed, causing all existing populations to be rebuilt into new Population objects (lines 76-80). This means that when retain_mut runs, it processes these newly created Population objects, but the new_populations vector created during the loop will never be merged back before being extended at line 188. The logic should either merge populations after all processing is complete, or the architecture needs to be restructured.
| if new_x >= 0 | ||
| && new_y >= 0 | ||
| && new_z >= 0 | ||
| && new_x < world.width as i32 | ||
| && new_y < world.height as i32 | ||
| && new_z < world.depth as i32 | ||
| { |
There was a problem hiding this comment.
Duplicating the is_valid check inline (lines 157-162) instead of calling world.is_valid() violates the DRY principle and creates maintainability issues. If the validation logic needs to change, it must be updated in multiple places. Consider using world.is_valid(new_x, new_y, new_z) instead.
| if new_x >= 0 | |
| && new_y >= 0 | |
| && new_z >= 0 | |
| && new_x < world.width as i32 | |
| && new_y < world.height as i32 | |
| && new_z < world.depth as i32 | |
| { | |
| if world.is_valid(new_x, new_y, new_z) { |
| // Diviser la population : une partie reste, l'autre se déplace | ||
| let moving_size = pop.size / 2; | ||
| if moving_size > 0 { | ||
| if moving_size > 10 { |
There was a problem hiding this comment.
The threshold for population movement has been changed from > 0 to > 10, which prevents small populations from ever migrating. This could cause populations of size 1-10 to be trapped in unsuitable locations, unable to search for better environments. This change may have unintended consequences for the simulation dynamics.
| if moving_size > 10 { | |
| if moving_size > 0 { |
| - name: Create GitHub Release | ||
| uses: actions/create-release@v1 | ||
| with: | ||
| tag_name: ${{ steps.version.outputs.new_tag }} | ||
| release_name: Release ${{ steps.version.outputs.new_tag }} | ||
| body: | | ||
| Stable release ${{ steps.version.outputs.new_tag }}. | ||
| draft: false | ||
| prerelease: false | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} No newline at end of file |
There was a problem hiding this comment.
The actions/create-release@v1 action is deprecated and will stop working soon. GitHub recommends using softprops/action-gh-release or the official GitHub CLI instead. See: https://github.blog/changelog/2024-02-29-deprecation-notice-v1-and-v2-of-the-artifact-actions/
|
errors in github actions closed PR |
No description provided.