Skip to content

Conversation

@louis-e
Copy link
Owner

@louis-e louis-e commented Jan 6, 2026

Motivation

  • Avoid cloning the potentially large Ground data when storing it in WorldEditor by sharing ownership with Arc.
  • Allow multiple parts of the generation pipeline to hold references to the same ground without copying.
  • Simplify memory management and reduce allocations when passing ground into the editor and other helpers.

Description

  • Change WorldEditor::ground from Option<Box<Ground>> to Option<Arc<Ground>> and add use std::sync::Arc.
  • Update set_ground to accept Arc<Ground> and store it directly, and update get_ground to return Option<&Ground> using as_deref().
  • In generate_world_with_options wrap the provided Ground in Arc::new(ground) and pass Arc::clone(&ground) to WorldEditor::set_ground, and update callers to use ground.as_ref() where a &Ground is required.
  • Add necessary use std::sync::Arc imports and adjust call sites in src/data_processing.rs to match the new API.

Testing

  • No automated tests were executed for this change.
  • Repository changes were committed locally and the modified files were updated without running a build or test suite.

Codex Task

Copilot AI review requested due to automatic review settings January 6, 2026 14:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the WorldEditor to use Arc<Ground> instead of Option<Box<Ground>> for the ground reference, eliminating unnecessary cloning of potentially large terrain elevation data. The change improves memory efficiency by enabling shared ownership of the Ground struct across multiple parts of the generation pipeline.

  • Replaced Box<Ground> with Arc<Ground> for efficient shared ownership
  • Updated set_ground API to accept Arc<Ground> directly instead of cloning from a reference
  • Simplified get_ground implementation using as_deref()

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/world_editor/mod.rs Changed ground field type to Arc<Ground>, updated setter to accept Arc directly, and simplified getter using as_deref()
src/data_processing.rs Wrapped ground in Arc::new(), passed cloned Arc to editor, and used as_ref() for function calls expecting &Ground

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link

github-actions bot commented Jan 6, 2026

⏱️ Benchmark run finished in 0m 30s
🧠 Peak memory usage: 825 MB

📈 Compared against baseline: 30s
🧮 Delta: 0s
🔢 Commit: cea8d89

🟢 Generation time is unchanged.

📅 Last benchmark: 2026-01-06 14:45:12 UTC

You can retrigger the benchmark by commenting retrigger-benchmark.

@louis-e louis-e merged commit cc89576 into main Jan 6, 2026
2 checks passed
@louis-e louis-e deleted the codex/change-ground-field-to-arcground branch January 6, 2026 15:09
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.

2 participants