Skip to content

Commit 84976f9

Browse files
gajopclaude
andcommitted
Give texture material discovery to the textures feature
list_materials / material_of / Material moved from textures/ui/model.rs to textures/materials.rs, reading the VFS directly rather than through the UI asset grid. The VFS listing helpers (vfs_files, join_entry, leaf, normalize_extensions) moved out of panels/controls/grid.rs to a neutral sbc/vfs.rs, so the domain no longer depends on panel UI. The now-orphaned ASSETS const is dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent e5a1444 commit 84976f9

9 files changed

Lines changed: 180 additions & 160 deletions

File tree

docs/porting/01-wip-refactor-plan.md

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ receives small, reviewable domain slices rather than architectural cleanup.
2424
under `panels/{controls,dialogs,cursor,fields,runtime,theme}`.
2525
- Duplicate model-factory registration now panics with the model's type name,
2626
as duplicate command registration already does (`Models::build`).
27+
- Texture material discovery is feature-owned: `Material` / `list_materials`
28+
moved to `textures/materials.rs`, reading the VFS directly. The VFS listing
29+
helpers moved out of the grid to a neutral `sbc/vfs.rs`, so the domain no
30+
longer depends on UI code.
2731

2832
## Remaining
2933

@@ -42,13 +46,6 @@ project-IO/workspace module (`io_registries`, `paths` exist but still sit inside
4246
project editor stay in `project/`. This is TODO #16; it stops each saving
4347
feature depending on the whole Project feature.
4448

45-
### Texture material ownership
46-
47-
`list_materials` / `material_of` still live in `textures/ui/model.rs`. Move
48-
material discovery/parsing into the `textures` feature proper; the editor
49-
consumes it. Texture/material ownership must not be trapped in a panel editor
50-
before the Textures domain transfers.
51-
5249
### Split the E2ERun monolith
5350

5451
`tools/e2e/runner.py` (~1150 lines) into engine-session lifecycle, input,

native/src/sbc/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,4 @@ mod message_handler;
3131
mod rml;
3232
pub mod sbc;
3333
mod tests;
34+
pub(crate) mod vfs;

native/src/sbc/panels/brush.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ pub(crate) fn non_empty(value: String) -> Option<String> {
2222
(!value.is_empty()).then_some(value)
2323
}
2424

25-
/// The direct thumbnail grids use full VFS paths. Asset fields instead pass a
26-
/// relative root to `AssetPicker`, which now defaults to this pack itself.
27-
pub(crate) const ASSETS: &str = "springboard/assets/core";
28-
2925
/// One action button: a caption, the image icon, and the brush it activates.
3026
#[derive(Clone, Copy)]
3127
pub(crate) struct BrushAction {

native/src/sbc/panels/controls/grid.rs

Lines changed: 1 addition & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::rc::Rc;
1414
use spring_native::prelude::{Error, NativeInterfaceRef};
1515

1616
use crate::sbc::panels::field::{bind_tooltip, bind_tooltip_markup, element_by_id, escape_rml};
17+
use crate::sbc::vfs::{join_entry, leaf, normalize_extensions, vfs_files};
1718

1819
/// Enough empty cells to fill out the widest row the panel can hold, so a short
1920
/// last row keeps its items at their natural size.
@@ -522,72 +523,10 @@ fn vfs_sub_dirs(interface: &NativeInterfaceRef, dir: &str) -> Vec<String> {
522523
names
523524
}
524525

525-
/// File names directly under `dir`, matching one of `extensions`. The VFS's own
526-
/// Extensions are matched as `.ext`, so a field may write either `png` or `.png`
527-
/// and both listers agree. They disagreed before, and a dotted list silently
528-
/// matched nothing.
529-
fn normalize_extensions(extensions: &[&str]) -> Vec<String> {
530-
extensions
531-
.iter()
532-
.map(|ext| ext.trim_start_matches('.').to_lowercase())
533-
.collect()
534-
}
535-
536-
/// `DirList`, as Lua's `Path.DirList` uses.
537-
fn vfs_files(interface: &NativeInterfaceRef, dir: &str, extensions: &[&str]) -> Vec<String> {
538-
let extensions = normalize_extensions(extensions);
539-
let Ok(paths) = interface.vfs().dir_list_names(dir, "*", "", false) else {
540-
return Vec::new();
541-
};
542-
let mut names: Vec<String> = paths
543-
.iter()
544-
.filter_map(|path| {
545-
let name = leaf(path)?;
546-
let matches = extensions.is_empty()
547-
|| extensions.iter().any(|ext| {
548-
name.to_lowercase()
549-
.ends_with(&format!(".{}", ext.to_lowercase()))
550-
});
551-
matches.then_some(name)
552-
})
553-
.collect();
554-
names.sort();
555-
names.dedup();
556-
names
557-
}
558-
559-
/// The last component of a VFS path, with any trailing slash dropped.
560-
fn leaf(path: &str) -> Option<String> {
561-
let name = path.trim_end_matches('/').rsplit('/').next()?.to_string();
562-
(!name.is_empty()).then_some(name)
563-
}
564-
565-
/// Join a directory with an entry the engine returned.
566-
///
567-
/// The engine hands back entries already prefixed with the directory, so
568-
/// joining unconditionally yields `bitmaps/bitmaps/foo.bmp` and the texture
569-
/// fails to load. Only join when the entry is a bare name.
570-
fn join_entry(dir: &str, name: &str) -> String {
571-
if dir.is_empty() || name.contains('/') {
572-
name.trim_end_matches('/').to_string()
573-
} else {
574-
format!("{}/{}", dir.trim_end_matches('/'), name)
575-
}
576-
}
577-
578526
#[cfg(test)]
579527
mod tests {
580528
use super::*;
581529

582-
#[test]
583-
fn entry_paths_are_not_doubled() {
584-
// The engine hands back `bitmaps/foo.bmp` inside `bitmaps`.
585-
assert_eq!(join_entry("bitmaps", "bitmaps/foo.bmp"), "bitmaps/foo.bmp");
586-
assert_eq!(join_entry("bitmaps", "foo.bmp"), "bitmaps/foo.bmp");
587-
assert_eq!(join_entry("", "foo.bmp"), "foo.bmp");
588-
assert_eq!(join_entry("a", "a/b/"), "a/b");
589-
}
590-
591530
#[test]
592531
fn parent_dir_walks_up_and_stops_at_the_root() {
593532
assert_eq!(parent_dir("a/b/c"), Some("a/b".to_string()));
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
//! Brush-material discovery: grouping the files under `brush_textures/` into
2+
//! materials by channel suffix. Owned by the textures feature; the editor UI
3+
//! consumes it.
4+
5+
use std::collections::BTreeMap;
6+
7+
use spring_native::prelude::NativeInterfaceRef;
8+
9+
use crate::sbc::vfs::{join_entry, vfs_files};
10+
11+
/// A material's channels, as `TextureManager.materialTextures` defines them: one
12+
/// texture per channel, found by suffix next to the diffuse.
13+
///
14+
/// `normal` is a channel of every material but has no enable toggle, exactly as
15+
/// Lua skips it when it builds the checkboxes.
16+
pub(crate) const CHANNELS: &[(&str, &str, bool)] = &[
17+
("diffuse", "Diffuse", true),
18+
("specular", "Specular", true),
19+
("normal", "Normal", false),
20+
("emission", "Emission", true),
21+
("refl", "Refl", true),
22+
];
23+
24+
const IMAGE_EXTS: &[&str] = &[".png", ".jpg", ".tga", ".dds", ".bmp"];
25+
26+
const ROOT: &str = "springboard/assets/core/brush_textures";
27+
28+
/// One material: its name, and the channel textures that exist for it.
29+
#[derive(Clone)]
30+
pub(crate) struct Material {
31+
/// The bare material name (`dirt1`), which is what the picker shows.
32+
pub(crate) name: String,
33+
pub(crate) channels: BTreeMap<String, String>,
34+
}
35+
36+
/// The material a texture belongs to: its file name with the channel suffix
37+
/// stripped, and without the directory. `.../brush_textures/dirt1_diffuse.png`
38+
/// is the `diffuse` of `dirt1`.
39+
fn material_of(path: &str) -> Option<(String, &'static str)> {
40+
let file = path.rsplit('/').next()?;
41+
let stem = file.rsplit_once('.').map(|(s, _)| s).unwrap_or(file);
42+
for (channel, _, _) in CHANNELS {
43+
if let Some(base) = stem.strip_suffix(&format!("_{channel}")) {
44+
return Some((base.to_string(), channel));
45+
}
46+
}
47+
None
48+
}
49+
50+
/// Group the files under `brush_textures/` into materials. A material exists if
51+
/// it has a diffuse; the other channels are optional, which is why the picker
52+
/// shows which ones were found.
53+
pub(crate) fn list_materials(interface: &NativeInterfaceRef) -> Vec<Material> {
54+
let mut found: BTreeMap<String, BTreeMap<String, String>> = BTreeMap::new();
55+
56+
for name in vfs_files(interface, ROOT, IMAGE_EXTS) {
57+
let path = join_entry(ROOT, &name);
58+
if let Some((material, channel)) = material_of(&path) {
59+
found
60+
.entry(material)
61+
.or_default()
62+
.insert(channel.to_string(), path);
63+
}
64+
}
65+
66+
found
67+
.into_iter()
68+
.filter(|(_, channels)| channels.contains_key("diffuse"))
69+
.map(|(name, channels)| Material { name, channels })
70+
.collect()
71+
}
72+
73+
#[cfg(test)]
74+
mod tests {
75+
use super::*;
76+
77+
/// The VFS hands back full paths, so the material's name is the file's, not
78+
/// the path's -- every material was captioned "springboard" until it was.
79+
#[test]
80+
fn a_material_is_named_after_its_file_not_its_path() {
81+
let (name, channel) =
82+
material_of("springboard/assets/core/brush_textures/dirt1_diffuse.png").unwrap();
83+
assert_eq!(name, "dirt1");
84+
assert_eq!(channel, "diffuse");
85+
86+
let (name, channel) =
87+
material_of("springboard/assets/core/brush_textures/cement_normal.png").unwrap();
88+
assert_eq!(name, "cement");
89+
assert_eq!(channel, "normal");
90+
}
91+
92+
#[test]
93+
fn a_texture_with_no_channel_suffix_belongs_to_no_material() {
94+
assert!(material_of("brush_textures/readme.png").is_none());
95+
}
96+
}

native/src/sbc/textures/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
pub(crate) mod commands;
2+
pub(crate) mod materials;
23
mod model;
34
pub(crate) mod ops;
45
mod project;

native/src/sbc/textures/ui/layout.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@ use crate::sbc::panels::editor_base::section_rml;
55
use crate::sbc::panels::runtime::Item;
66
use crate::sbc::rml::{element_by_id, escape_rml};
77

8+
use crate::sbc::textures::materials::list_materials;
9+
810
use super::model::TexField::*;
911
use super::model::{
10-
enabled_name, list_materials, material_tooltip, toggle_channels, TexField, TextureUiModel,
11-
ADD_BRUSH_ID,
12+
enabled_name, material_tooltip, toggle_channels, TexField, TextureUiModel, ADD_BRUSH_ID,
1213
};
1314

1415
fn section_markup(id: &str, caption: &str) -> String {

native/src/sbc/textures/ui/model.rs

Lines changed: 3 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,16 @@ use std::cell::RefCell;
44
use std::collections::BTreeMap;
55
use std::rc::Rc;
66

7-
use crate::sbc::panels::brush::{non_empty, BrushAction, BrushActions, ASSETS};
8-
use crate::sbc::panels::controls::grid::{list_assets, GridView};
7+
use crate::sbc::panels::brush::{non_empty, BrushAction, BrushActions};
8+
use crate::sbc::panels::controls::grid::GridView;
99
use crate::sbc::panels::field::FieldValue;
1010
use crate::sbc::panels::fields::{BooleanField, ChoiceField, ColorField, NumericField};
1111
use crate::sbc::panels::runtime::{
1212
AssetGrid, AssetGridDef, Brush, EditorModel, FieldMut, FieldRef, TableEntry, TableModel,
1313
};
1414
use crate::sbc::rml::escape_rml;
1515
use crate::sbc::states::{BrushKind, BrushSettings};
16+
use crate::sbc::textures::materials::{Material, CHANNELS};
1617

1718
/// Blend modes, in the order `texture_editor.lua` lists them.
1819
const MODES: &[&str] = &[
@@ -74,21 +75,6 @@ pub(super) const ACTIONS: &[BrushAction] = &[
7475
},
7576
];
7677

77-
/// A material's channels, as `TextureManager.materialTextures` defines them: one
78-
/// texture per channel, found by suffix next to the diffuse.
79-
///
80-
/// `normal` is a channel of every material but has no enable toggle, exactly as
81-
/// Lua skips it when it builds the checkboxes.
82-
const CHANNELS: &[(&str, &str, bool)] = &[
83-
("diffuse", "Diffuse", true),
84-
("specular", "Specular", true),
85-
("normal", "Normal", false),
86-
("emission", "Emission", true),
87-
("refl", "Refl", true),
88-
];
89-
90-
const IMAGE_EXTS: &[&str] = &[".png", ".jpg", ".tga", ".dds", ".bmp"];
91-
9278
/// The engine exposes at most four DNTS (splat normal) channels.
9379
pub(super) const DNTS_COUNT: i32 = 4;
9480

@@ -99,14 +85,6 @@ pub(super) fn toggle_channels() -> impl Iterator<Item = &'static str> {
9985
.map(|(channel, _, _)| *channel)
10086
}
10187

102-
/// One material: its name, and the channel textures that exist for it.
103-
#[derive(Clone)]
104-
pub(super) struct Material {
105-
/// The bare material name (`dirt1`), which is what the picker shows.
106-
pub(super) name: String,
107-
pub(super) channels: BTreeMap<String, String>,
108-
}
109-
11088
pub(super) struct SavedBrush {
11189
pub(super) id: String,
11290
pub(super) material: String,
@@ -115,46 +93,6 @@ pub(super) struct SavedBrush {
11593

11694
pub(super) const ADD_BRUSH_ID: &str = "__add_saved_brush__";
11795

118-
/// The material a texture belongs to: its file name with the channel suffix
119-
/// stripped, and without the directory. `.../brush_textures/dirt1_diffuse.png`
120-
/// is the `diffuse` of `dirt1`.
121-
fn material_of(path: &str) -> Option<(String, &'static str)> {
122-
let file = path.rsplit('/').next()?;
123-
let stem = file.rsplit_once('.').map(|(s, _)| s).unwrap_or(file);
124-
for (channel, _, _) in CHANNELS {
125-
if let Some(base) = stem.strip_suffix(&format!("_{channel}")) {
126-
return Some((base.to_string(), channel));
127-
}
128-
}
129-
None
130-
}
131-
132-
/// Group the files under `brush_textures/` into materials. A material exists if
133-
/// it has a diffuse; the other channels are optional, which is why the picker
134-
/// shows which ones were found.
135-
pub(super) fn list_materials(interface: &NativeInterfaceRef) -> Vec<Material> {
136-
let root = format!("{ASSETS}/brush_textures");
137-
let mut found: BTreeMap<String, BTreeMap<String, String>> = BTreeMap::new();
138-
139-
for item in list_assets(interface, &root, IMAGE_EXTS) {
140-
if item.is_directory {
141-
continue;
142-
}
143-
if let Some((name, channel)) = material_of(&item.id) {
144-
found
145-
.entry(name)
146-
.or_default()
147-
.insert(channel.to_string(), item.id.clone());
148-
}
149-
}
150-
151-
found
152-
.into_iter()
153-
.filter(|(_, channels)| channels.contains_key("diffuse"))
154-
.map(|(name, channels)| Material { name, channels })
155-
.collect()
156-
}
157-
15896
pub(super) fn material_tooltip(material: &Material) -> String {
15997
let channel = |name: &str, title: &str| {
16098
let (color, mark) = if material.channels.contains_key(name) {
@@ -677,26 +615,6 @@ pub(super) fn enabled_name(channel: &str) -> String {
677615
mod tests {
678616
use super::*;
679617

680-
/// The VFS hands back full paths, so the material's name is the file's, not
681-
/// the path's -- every material was captioned "springboard" until it was.
682-
#[test]
683-
fn a_material_is_named_after_its_file_not_its_path() {
684-
let (name, channel) =
685-
material_of("springboard/assets/core/brush_textures/dirt1_diffuse.png").unwrap();
686-
assert_eq!(name, "dirt1");
687-
assert_eq!(channel, "diffuse");
688-
689-
let (name, channel) =
690-
material_of("springboard/assets/core/brush_textures/cement_normal.png").unwrap();
691-
assert_eq!(name, "cement");
692-
assert_eq!(channel, "normal");
693-
}
694-
695-
#[test]
696-
fn a_texture_with_no_channel_suffix_belongs_to_no_material() {
697-
assert!(material_of("brush_textures/readme.png").is_none());
698-
}
699-
700618
#[test]
701619
fn normal_is_a_channel_but_has_no_toggle() {
702620
let toggles: Vec<&str> = toggle_channels().collect();

0 commit comments

Comments
 (0)