Skip to content

no threads in wasm#41

Open
lukacslacko wants to merge 3 commits intomainfrom
update-wasm
Open

no threads in wasm#41
lukacslacko wants to merge 3 commits intomainfrom
update-wasm

Conversation

@lukacslacko
Copy link
Copy Markdown
Owner

No description provided.

@lukacslacko lukacslacko requested review from Copilot and papjuli June 7, 2025 12:58
Copy link
Copy Markdown
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 updates the application to disable thread usage when running in a WASM environment and makes ancillary adjustments in configuration defaults, wasm bindings, and the documentation assets.

  • In src/ui.rs, the globe generation code now runs synchronously for wasm using conditional compilation.
  • In src/state.rs, default configuration values for grid size and perlin seed have been updated.
  • In docs/terrain3d.js and docs/index.html, asset paths and wasm binding adapter names are updated and a new overlay is implemented for build failure notifications.

Reviewed Changes

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

File Description
src/ui.rs Introduces conditional compilation to bypass thread spawn in wasm.
src/state.rs Adjusts default config parameters (grid_size and perlin seed).
docs/terrain3d.js Updates wasm binding adapter names for consistency.
docs/index.html Revises asset paths and adds overlay UI code for build failure.

docs/index.html Outdated
Comment on lines +79 to +81
window.setInterval(() => {
this._inject();
}, 250);
Copy link

Copilot AI Jun 7, 2025

Choose a reason for hiding this comment

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

Using a setInterval every 250ms to repeatedly inject the overlay could impact performance; consider using more event-driven methods like a MutationObserver or specific event listeners to handle overlay reinjection.

Suggested change
window.setInterval(() => {
this._inject();
}, 250);
// Use MutationObserver to monitor DOM changes
const observer = new MutationObserver(() => {
this._inject();
});
observer.observe(document.body, { childList: true });

Copilot uses AI. Check for mistakes.
docs/index.html Outdated
class Overlay {
constructor() {
// create an overlay
this._overlay = document.createElement("div");
Copy link

Copilot AI Jun 7, 2025

Choose a reason for hiding this comment

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

Consider adding an appropriate ARIA role attribute (e.g., role="dialog" or role="alert") to the overlay element to improve accessibility for users with assistive technologies.

Suggested change
this._overlay = document.createElement("div");
this._overlay = document.createElement("div");
this._overlay.setAttribute("role", "alert");

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@papjuli papjuli left a comment

Choose a reason for hiding this comment

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

lgtm

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.

3 participants