From 29275c38fc7bbe4201d7634af1d292c71afa1047 Mon Sep 17 00:00:00 2001 From: Steve Biedermann Date: Wed, 21 Oct 2026 22:20:13 +0200 Subject: [PATCH] fix(tiled): enforce tmx exports and group objects by category Normalize export filenames to .tmx and return the actual saved path in command replies. Separate floors, structures, ground items and spawn points into named object layers, with a reserved wiring/plumbing layer. Reuse the same categories in the Tiled converter and preserve undo behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 3 + README.md | 13 +- assets/tiled/station.js | 30 ++- crates/game_server/src/commands.rs | 4 +- crates/game_server/src/map_io.rs | 246 ++++++++++++++++++++----- crates/game_server/tests/simulation.rs | 9 +- tools/tiled.test.cjs | 52 +++++- 7 files changed, 297 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc5d94c..a372a0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,9 @@ based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed +- Tiled exports always use the `.tmx` extension and separate floors, structures, + ground items and spawn points into named object layers, with a reserved + wiring/plumbing layer. The Tiled converter uses the same layer organization. - Tiled property overrides apply to new-world seeds as well as imported maps. Terrain is persisted with world state and replicated to connected clients. Save databases migrate automatically to schema version 2. diff --git a/README.md b/README.md index 0b07a2d..bdb72a7 100644 --- a/README.md +++ b/README.md @@ -417,7 +417,9 @@ copy `station.js` there, then enable/reload it (or restart Tiled). Paint the marked tiles normally, then choose **Map > Station: Convert Painted Tiles to Objects**. The extension replaces all marked painted cells with selected -`building` point objects in a new **Station gameplay** object layer. Select one +`building` point objects in separate **Floors** and **Objects** layers. Existing +layers with those names are reused. The converter also creates **Wiring/Plumbing**, +**Ground Items** and **Triggers** layers when missing. Select an object to edit its health, solidity or other custom properties in the Properties panel. Unmarked terrain stays painted. Floor/structure overlaps are allowed; same-layer overlaps abort the entire conversion. Conversion is a single undoable operation, @@ -439,11 +441,20 @@ and coordinate conventions; conversion is optional before loading a map. beside the database. With PostgreSQL it defaults to `saves//world.tmx`; without saving, `saves/world.tmx`. An explicit path, including spaces, may follow either command; relative paths are relative to the server's working directory. + Exports always use `.tmx`: a missing extension is added and any other extension + is replaced (for example, `/export repair.backup` saves `repair.tmx`). The reply + reports the actual filename; use that filename when importing. 2. Open that file in Tiled, edit it and save. Buildings and ground items are point objects with their prototype and full mutable state in custom properties. Terrain layers and collision properties are included. Tilesets are embedded with absolute image references into the server's assets directory; when editing on another machine, relink the images to that machine's matching assets. + Objects are organized into **Floors**, **Objects** (walls, machines and other + structures), **Ground Items**, and **Triggers** (currently spawn points). + **Wiring/Plumbing** is an empty organizational placeholder; it does not add + wiring, plumbing or trigger mechanics. Hide or lock individual layers in Tiled + to edit one category without the others getting in the way. Object classes and + properties determine gameplay, not the editor layer names. 3. Run `/import` (or `/import FILE`). **Import replaces all terrain, buildings and ground items, including deletions.** It does not merge objects or change online or offline characters, their positions or inventories. Export a separate backup diff --git a/assets/tiled/station.js b/assets/tiled/station.js index fdf9eab..63ac36a 100644 --- a/assets/tiled/station.js +++ b/assets/tiled/station.js @@ -103,10 +103,28 @@ function stationConvert(map) { if (planned.length === 0) return 0; + const names = ["Floors", "Wiring/Plumbing", "Objects", "Ground Items", "Triggers"]; + const destinations = new Map(); + for (const name of names) { + const matches = layers.filter(layer => layer.isObjectLayer && layer.name === name); + if (matches.length > 1) + throw new Error("Multiple object layers named " + name + "; rename them before converting."); + if (matches.length === 1) { + if (matches[0].locked) + throw new Error("Unlock object layer before converting: " + name); + destinations.set(name, matches[0]); + } + } map.macro("Convert Station painted tiles to objects", function () { - const objects = new ObjectGroup(); - objects.name = "Station gameplay"; - map.addLayer(objects); + for (const name of names) { + if (!destinations.has(name)) { + const objects = new ObjectGroup(); + objects.name = name; + map.addLayer(objects); + destinations.set(name, objects); + } + } + const selected = []; const edits = new Map(); for (const entry of planned) { const object = new MapObject(); @@ -116,14 +134,16 @@ function stationConvert(map) { object.x = (entry.x + 0.5) * map.tileWidth; object.y = (entry.y + 0.5) * map.tileHeight; object.setProperties(entry.properties); - objects.addObject(object); + const name = stationLayer(entry.properties, prototypeLayers) === "floor" ? "Floors" : "Objects"; + destinations.get(name).addObject(object); + selected.push(object); if (!edits.has(entry.layer)) edits.set(entry.layer, entry.layer.edit()); edits.get(entry.layer).setTile(entry.x, entry.y, null); } for (const edit of edits.values()) edit.apply(); - map.selectedObjects = objects.objects; + map.selectedObjects = selected; }); return planned.length; } diff --git a/crates/game_server/src/commands.rs b/crates/game_server/src/commands.rs index 712439d..11f0b28 100644 --- a/crates/game_server/src/commands.rs +++ b/crates/game_server/src/commands.rs @@ -229,10 +229,10 @@ pub fn execute(world: &mut World, source: CommandSource, line: &str) -> Vec { + Ok(path) => { info!("{actor}: {command} {}", path.display()); let mut message = format!("World {command}ed: {}", path.display()); if command == "import" && !world.contains_resource::() { diff --git a/crates/game_server/src/map_io.rs b/crates/game_server/src/map_io.rs index 7f76974..f1d7976 100644 --- a/crates/game_server/src/map_io.rs +++ b/crates/game_server/src/map_io.rs @@ -181,7 +181,7 @@ pub fn default_path(world: &World) -> PathBuf { ) } -pub fn export(world: &mut World, path: &Path) -> Result<(), MapIoError> { +pub fn export(world: &mut World, path: &Path) -> Result { let mut save = persistence::snapshot(world); let data = world.resource::(); if let Some(layout) = &mut save.layout { @@ -311,7 +311,8 @@ fn write_properties(xml: &mut Xml, fields: &BTreeMap) -> Result<( Ok(()) } -pub fn write(save: &SaveGame, path: &Path) -> Result<(), MapIoError> { +pub fn write(save: &SaveGame, path: &Path) -> Result { + let path = path.with_extension("tmx"); let layout = save .layout .as_ref() @@ -442,67 +443,89 @@ pub fn write(save: &SaveGame, path: &Path) -> Result<(), MapIoError> { end(&mut xml, "data")?; end(&mut xml, "layer")?; } - tag( - &mut xml, - "objectgroup", - &[ - ("id", (layout.layers.len() + 1).to_string()), - ("name", "World".into()), - ], - false, - )?; let mut id = 1; - for object in layout.objects.iter().filter(|o| o.class == "spawn") { - write_object( + for (index, name) in [ + "Floors", + "Wiring/Plumbing", + "Objects", + "Ground Items", + "Triggers", + ] + .iter() + .enumerate() + { + tag( &mut xml, - id, - "spawn", - &object.name, - object.pos, - layout, - &BTreeMap::new(), + "objectgroup", + &[ + ("id", (layout.layers.len() + index + 1).to_string()), + ("name", (*name).into()), + ("draworder", "topdown".into()), + ], + false, )?; - id += 1; - } - for b in &save.buildings { - let mut fields = properties(&b.state, "state.")?; - fields.insert("proto".into(), Value::String(b.proto.clone())); - if let Some(machine) = &b.machine { - fields.extend(properties(machine, "machine.")?); + for b in save.buildings.iter().filter(|b| { + if b.state.layer == game_core::data::BuildingLayer::Floor { + *name == "Floors" + } else { + *name == "Objects" + } + }) { + let mut fields = properties(&b.state, "state.")?; + fields.insert("proto".into(), Value::String(b.proto.clone())); + if let Some(machine) = &b.machine { + fields.extend(properties(machine, "machine.")?); + } + write_object(&mut xml, id, "building", &b.proto, b.pos, layout, &fields)?; + id += 1; } - write_object(&mut xml, id, "building", &b.proto, b.pos, layout, &fields)?; - id += 1; + if *name == "Ground Items" { + for item in &save.items { + let fields = BTreeMap::from([ + ("proto".into(), Value::String(item.stack.item.clone())), + ("count".into(), Value::from(item.stack.count)), + ]); + write_object( + &mut xml, + id, + "item", + &item.stack.item, + item.pos, + layout, + &fields, + )?; + id += 1; + } + } + if *name == "Triggers" { + for object in layout.objects.iter().filter(|o| o.class == "spawn") { + write_object( + &mut xml, + id, + "spawn", + &object.name, + object.pos, + layout, + &BTreeMap::new(), + )?; + id += 1; + } + } + end(&mut xml, "objectgroup")?; } - for item in &save.items { - let fields = BTreeMap::from([ - ("proto".into(), Value::String(item.stack.item.clone())), - ("count".into(), Value::from(item.stack.count)), - ]); - write_object( - &mut xml, - id, - "item", - &item.stack.item, - item.pos, - layout, - &fields, - )?; - id += 1; - } - end(&mut xml, "objectgroup")?; end(&mut xml, "map")?; if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) { std::fs::create_dir_all(parent)?; } let temporary = path.with_extension(format!("{}.tmp", uuid::Uuid::new_v4())); std::fs::write(&temporary, xml.into_inner())?; - if let Err(error) = std::fs::rename(&temporary, path) { + if let Err(error) = std::fs::rename(&temporary, &path) { if let Err(cleanup) = std::fs::remove_file(&temporary) { warn!("failed to remove {}: {cleanup}", temporary.display()); } return Err(error.into()); } - Ok(()) + Ok(path) } fn write_object( @@ -554,6 +577,130 @@ mod tests { std::env::temp_dir().join(format!("station-map-{}.tmx", uuid::Uuid::new_v4())) } + #[test] + fn writer_uses_tmx_extension_and_separates_object_categories() { + let (_, save) = seeded(); + for extension in ["", "backup", "TMX", "tmx"] { + let requested = path().with_extension(extension); + let saved = write(&save, &requested).unwrap(); + assert_eq!(saved, requested.with_extension("tmx")); + assert!(saved.is_file()); + if requested != saved { + assert!(!requested.exists()); + } + let mut reader = quick_xml::Reader::from_file(&saved).unwrap(); + let mut buffer = Vec::new(); + let mut groups = BTreeMap::::new(); + let mut current = None; + let mut layer_ids = BTreeSet::new(); + let mut object_ids = BTreeSet::new(); + loop { + match reader.read_event_into(&mut buffer).unwrap() { + Event::Start(element) if element.name().as_ref() == b"objectgroup" => { + let mut name = None; + for attribute in element.attributes() { + let attribute = attribute.unwrap(); + let value = attribute + .decoded_and_normalized_value( + quick_xml::XmlVersion::Implicit1_0, + reader.decoder(), + ) + .unwrap() + .into_owned(); + match attribute.key.as_ref() { + b"name" => name = Some(value), + b"id" => assert!(layer_ids.insert(value)), + _ => {} + } + } + let name = name.unwrap(); + assert!(groups.insert(name.clone(), 0).is_none()); + current = Some(name); + } + Event::Start(element) if element.name().as_ref() == b"object" => { + *groups.get_mut(current.as_ref().unwrap()).unwrap() += 1; + for attribute in element.attributes() { + let attribute = attribute.unwrap(); + if attribute.key.as_ref() == b"id" { + assert!( + object_ids.insert( + attribute + .decoded_and_normalized_value( + quick_xml::XmlVersion::Implicit1_0, + reader.decoder() + ) + .unwrap() + .into_owned() + ) + ); + } + } + } + Event::End(element) if element.name().as_ref() == b"objectgroup" => { + current = None + } + Event::Empty(element) if element.name().as_ref() == b"property" => { + let attributes: BTreeMap<_, _> = element + .attributes() + .map(|attribute| { + let attribute = attribute.unwrap(); + ( + attribute.key.as_ref().to_vec(), + attribute + .decoded_and_normalized_value( + quick_xml::XmlVersion::Implicit1_0, + reader.decoder(), + ) + .unwrap() + .into_owned(), + ) + }) + .collect(); + if attributes + .get(b"name".as_slice()) + .is_some_and(|name| name == "state.layer") + && current.is_some() + { + let expected = if attributes[b"value".as_slice()] == "\"floor\"" { + "Floors" + } else { + "Objects" + }; + assert_eq!(current.as_deref(), Some(expected)); + } + } + Event::Eof => break, + _ => {} + } + buffer.clear(); + } + let floors = save + .buildings + .iter() + .filter(|b| b.state.layer == game_core::data::BuildingLayer::Floor) + .count(); + assert!(floors > 0); + assert_eq!( + groups, + BTreeMap::from([ + ("Floors".into(), floors), + ("Wiring/Plumbing".into(), 0), + ("Objects".into(), save.buildings.len() - floors), + ("Ground Items".into(), save.items.len()), + ( + "Triggers".into(), + save.layout + .as_ref() + .unwrap() + .objects_of_class("spawn") + .count() + ), + ]) + ); + std::fs::remove_file(saved).unwrap(); + } + } + fn fixture(layers: &str, properties: &str) -> Result { let file = path(); let tileset = game_core::assets_dir().join("tilesets/station.tsx"); @@ -757,7 +904,10 @@ mod tests { let file = path(); write(&save, &file).unwrap(); let loaded = MapLayout::load(&game_core::assets_dir(), file.to_str().unwrap()).unwrap(); - let roundtrip = decode(loaded, &data).unwrap(); + let mut roundtrip = decode(loaded, &data).unwrap(); + let key = |b: &SavedBuilding| (b.state.layer, b.pos.x, b.pos.y); + roundtrip.buildings.sort_by_key(key); + save.buildings.sort_by_key(key); assert_eq!(roundtrip.buildings, save.buildings); assert_eq!(roundtrip.items, save.items); let loaded = roundtrip.layout.unwrap(); diff --git a/crates/game_server/tests/simulation.rs b/crates/game_server/tests/simulation.rs index 2bc8c1b..0b34f4a 100644 --- a/crates/game_server/tests/simulation.rs +++ b/crates/game_server/tests/simulation.rs @@ -96,9 +96,11 @@ fn admin_tiled_import_replaces_world_preserves_characters_and_rejects_invalid_ma let reply = commands::execute( app.world_mut(), CommandSource::Console, - &format!("export {filename}"), + &format!("export {}", file.with_extension("backup").display()), ); assert!(reply[0].contains("exported"), "{reply:?}"); + assert!(reply[0].ends_with(filename), "{reply:?}"); + assert!(!file.with_extension("backup").exists()); let original = std::fs::read_to_string(&file).unwrap(); assert!( !original.contains("MapAdmin"), @@ -182,11 +184,14 @@ fn admin_tiled_import_replaces_world_preserves_characters_and_rejects_invalid_ma map_io::write(&expected, &file).unwrap(); map_io::import(app.world_mut(), &file).unwrap(); let storage = app.world().resource::(); - let persisted = storage + let mut persisted = storage .runtime .block_on(storage.db.load_world(storage.world.id)) .unwrap(); expected.characters = imported.characters; + let key = |b: &persistence::SavedBuilding| (b.state.layer, b.pos.x, b.pos.y); + persisted.buildings.sort_by_key(key); + expected.buildings.sort_by_key(key); assert_eq!(persisted.buildings, expected.buildings); assert_eq!(persisted.items, expected.items); assert_eq!(persisted.characters, expected.characters); diff --git a/tools/tiled.test.cjs b/tools/tiled.test.cjs index 195ef62..a9bc945 100644 --- a/tools/tiled.test.cjs +++ b/tools/tiled.test.cjs @@ -51,10 +51,13 @@ function harness() { const layers = [...this.layers]; const cells = layers.filter(layer => layer.isTileLayer) .map(layer => [layer, new Map(layer.cells)]); + const objects = layers.filter(layer => layer.isObjectLayer) + .map(layer => [layer, [...layer.objects]]); callback(); this.undo = () => { this.layers = layers; for (const [layer, snapshot] of cells) layer.cells = snapshot; + for (const [layer, snapshot] of objects) layer.objects = snapshot; }; }, }; @@ -98,7 +101,12 @@ test("converts floors and walls together, retaining properties, flags and decora map.layers.push(ground, wall); assert.equal(context.stationConvert(map), 2); assert.equal(map.macros, 1); - const objects = map.layers[2].objects; + assert.deepEqual(map.layers.slice(2).map(layer => layer.name), + ["Floors", "Wiring/Plumbing", "Objects", "Ground Items", "Triggers"]); + const objects = [map.layers[2].objects[0], map.layers[4].objects[0]]; + assert.equal(map.layers[2].objects.length, 1); + assert.equal(map.layers[3].objects.length, 0); + assert.equal(map.layers[4].objects.length, 1); assert.equal(objects[0].className, "building"); assert.equal(objects[0].shape, "point"); assert.equal(objects[0].x, 16); @@ -115,7 +123,7 @@ test("converts floors and walls together, retaining properties, flags and decora assert.equal(ground.tileAt(0, 0), null); assert.notEqual(ground.tileAt(1, 0), null); assert.equal(wall.tileAt(0, 0), null); - assert.equal(map.selectedObjects, objects); + assert.deepEqual([...map.selectedObjects], objects); assert.equal(context.stationConvert(map), 0); assert.equal(map.macros, 1); map.undo(); @@ -124,6 +132,46 @@ test("converts floors and walls together, retaining properties, flags and decora assert.notEqual(wall.tileAt(0, 0), null); }); +test("reuses categorized object layers and undoes additions without changing existing objects", () => { + const {map, context} = harness(); + const floors = new context.ObjectGroup(); + floors.name = "Floors"; + const existing = {className: "building", x: 48, y: 48, tile: null, + properties: () => ({proto: "floor", "state.layer": '"floor"'})}; + floors.addObject(existing); + const objects = new context.ObjectGroup(); + objects.name = "Objects"; + map.layers.push(floors, objects, layer([[0, 0, tile({proto: "floor", "state.layer": '"floor"'})]])); + assert.equal(context.stationConvert(map), 1); + assert.equal(map.layers.filter(layer => layer.name === "Floors").length, 1); + assert.equal(floors.objects.length, 2); + assert.equal(floors.objects[0], existing); + assert.equal(objects.objects.length, 0); + map.undo(); + assert.equal(floors.objects.length, 1); + assert.equal(floors.objects[0], existing); + assert.equal(map.layers.length, 3); +}); + +test("refuses ambiguous or locked destination layers before modifying the map", () => { + for (const duplicate of [false, true]) { + const {map, context} = harness(); + const floors = new context.ObjectGroup(); + floors.name = "Floors"; + map.layers.push(floors, layer([[0, 0, tile({proto: "floor", "state.layer": '"floor"'})]])); + if (duplicate) { + const second = new context.ObjectGroup(); + second.name = "Floors"; + map.layers.push(second); + } else { + floors.locked = true; + } + assert.throws(() => context.stationConvert(map), duplicate ? /Multiple object layers/ : /Unlock object layer/); + assert.equal(map.macros, 0); + assert.equal(floors.objects.length, 0); + } +}); + test("rejects same-layer duplicates before making changes", () => { for (const properties of [{proto: "wall"}, {proto: "floor", "state.layer": '"floor"'}]) { const {map, context} = harness();