Skip to content

fix: tileMove-based functions on a level tilemap #1078

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.

Sign up for GitHub

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

Open
SobennaStory wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: tileMove-based functions on a level tilemap #1078

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
57ddd49
fix: tileMove-based functions on a level tilemap
SobennaStory Apr 20, 2026
99cda4d
fix: fixing formatting errors and insert/remove automatically creates…
SobennaStory Apr 21, 2026
f105c5b
chore: run fmt
SobennaStory Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions src/ecs/components/level/level.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@ export interface LevelCompOpt {
tiles: {
[sym: string]: (pos: Vec2) => CompList<Comp>;
};
/**
* Enable spatial map after adding level tiles
*/
spatialMap?: boolean;
/**
* Enable cost, connectivity, and edge maps after adding level tiles
*/
tilePathMaps?: boolean;

@dragoncoder047 dragoncoder047 Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/**
* Enable spatial map after adding level tiles
*/
spatialMap?: boolean;
/**
* Enable cost, connectivity, and edge maps after adding level tiles
*/
tilePathMaps?: boolean;

/**
* Called when encountered a symbol not defined in "tiles".
*/
Expand Down Expand Up @@ -149,7 +157,16 @@ export function level(map: string[], opt: LevelCompOpt): LevelComp {
}
};

const ensureSpatialMapExists = () => {
if (!spatialMap) {
throw new Error(
"Spatial map is not initialized. Call level.getSpatialMap() before using spatial map functions, or enable the 'spatialMap' option in level().",
);
}
};

@dragoncoder047 dragoncoder047 Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
const ensureSpatialMapExists = () => {
if (!spatialMap) {
throw new Error(
"Spatial map is not initialized. Call level.getSpatialMap() before using spatial map functions, or enable the 'spatialMap' option in level().",
);
}
};
const ensureSpatialMapExists = () => {
if (!spatialMap) {
createSpatialMap(this);
}
};

If the spatial map doesn't exist yet ...why not just create it now?

@SobennaStory SobennaStory Apr 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The this inside an arrow function is lexically bound, so it won't refer to the level instance here.
The throw approach sidesteps the problem by making it the caller's responsibility to initialize the map first.

I also mused on

  1. Simply creating the spatial map when the component is attached via add()
  2. Making the tile.tilemove functions throw errors if no spatial map is found in their responsible level
  3. Creating a spatial map whenever level.spawn() is called.

What do you think the best approach is? To implement the above suggestion we'd need to change the props of insertIntoSpatialMap and removeFromSpatialMap which may break other usages of the functions?

@dragoncoder047 dragoncoder047 Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shoot, forgot about that part, that's what I get for not checking this out locally.

to your muses

  1. not necessary for levels that don't use it
  2. please no errors when it could just be created
  3. not necessary for levels that don't use it

I mostly just wanted to reduce the number of edits, so just every place that ensureSpatialMapExists() is called, just replace it with if (!spatialMap) createSpatialMap(this) or if (!spatialMap) createSpatialMap(obj), you might have to make wrappers for insertIntoSpacialMap and removeFromSpatialMap, instead of just using the functions directly, make them wrappers to get the this -

-   insertIntoSpatialMap,
+   insertIntoSpatialMap(obj) { insertIntoSpatialMap(obj, this); }

const insertIntoSpatialMap = (obj: GameObj) => {
ensureSpatialMapExists();
const i = tile2Hash(obj.tilePos);
if (spatialMap![i]) {
spatialMap![i].push(obj);
Expand All @@ -160,6 +177,7 @@ export function level(map: string[], opt: LevelCompOpt): LevelComp {
};

const removeFromSpatialMap = (obj: GameObj) => {
ensureSpatialMapExists();
const i = tile2Hash(obj.tilePos);
if (spatialMap![i]) {
const index = spatialMap![i].indexOf(obj);
Expand Down Expand Up @@ -351,6 +369,16 @@ export function level(map: string[], opt: LevelCompOpt): LevelComp {
this.spawn(key, vec2(j, i));
});
});

if (opt.spatialMap) {
createSpatialMap(this);
}

if (opt.tilePathMaps) {
createCostMap(this);
createEdgeMap(this);
createConnectivityMap(this);
}

@dragoncoder047 dragoncoder047 Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (opt.spatialMap) {
createSpatialMap(this);
}
if (opt.tilePathMaps) {
createCostMap(this);
createEdgeMap(this);
createConnectivityMap(this);
}

},

tileWidth() {
Expand Down Expand Up @@ -737,9 +765,9 @@ export function levelFactory(data: any) {
const d = data.tiles[key];
const tags = d.tags;
opt.tiles[key] = (pos: Vec2) => {
const comps: Comp[] = Object.keys(d).filter(k => k != "tags").map(
id => deserializeComp(id, d[id]),
);
const comps: Comp[] = Object.keys(d)
.filter((k) => k != "tags")
.map((id) => deserializeComp(id, d[id]));
Comment thread
SobennaStory marked this conversation as resolved.
return [...comps, ...tags];
};
}
Expand Down