Path cache file creation bugfix #140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
melody-rs
requested changes
Jul 15, 2024
There was a problem hiding this comment.
Removing files seems to still not work:
The application panicked (crashed).
Message: called `Option::unwrap()` on a `None` value
Location: /home/lily/Git/Luminol/crates/filesystem/src/path_cache.rs:495
Luminol version: f011b25-modified
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
⋮ 11 frames hidden ⋮
12: <luminol_filesystem::path_cache::FileSystem<F> as luminol_filesystem::FileSystem>::remove_file::h40d6e2e4e87cd374
at /home/lily/Git/Luminol/crates/filesystem/src/path_cache.rs:495
493 │ {
494 │ let cache = &mut *cache;
495 > for extension_trie in cache.trie.iter_prefix(&path).unwrap().map(|(_, t)| t) {
496 │ for index in extension_trie.values().copied() {
497 │ cache.cactus.remove(index);
13: <luminol_filesystem::project::FileSystem as luminol_filesystem::FileSystem>::remove_file::hec152ff3da640f15
at /home/lily/Git/Luminol/crates/filesystem/src/project.rs:713
711 │ FileSystem::Unloaded => Err(Error::NotLoaded.into()),
712 │ FileSystem::HostLoaded(f) => f.remove_file(path),
713 > FileSystem::Loaded { filesystem, .. } => filesystem.remove_file(path),
714 │ }
715 │ }
14: luminol_core::data_cache::data_formats::Handler::remove_file::h7ff6154bc4f111f7
at /home/lily/Git/Luminol/crates/core/src/data_cache/data_formats.rs:218
216 │ .join(filename)
217 │ .with_extension(self.format.extension());
218 > filesystem.remove_file(path)
219 │ }
220 │ }
15: luminol_core::data_cache::Data::convert_project::h65172b47a9da2556
at /home/lily/Git/Luminol/crates/core/src/data_cache.rs:402
400 │
401 │ to_handler.write_nil_padded(&actors.get_mut().data, filesystem, "Actors")?;
402 > from_handler.remove_file(filesystem, "Actors")?;
403 │
404 │ to_handler.write_nil_padded(&animations.get_mut().data, filesystem, "Animations")?;
16: <luminol_ui::windows::config_window::Window as luminol_core::window::Window>::show::{{closure}}::{{closure}}::h4f27f1c485575bd4
at /home/lily/Git/Luminol/crates/ui/src/windows/config_window.rs:129
127 │ .clicked();
128 │ if clicked {
129 > update_state
130 │ .data
131 │ .convert_project(
⋮ 5 frames hidden ⋮
22: <luminol_ui::windows::config_window::Window as luminol_core::window::Window>::show::{{closure}}::h67d4828073151261
at /home/lily/Git/Luminol/crates/ui/src/windows/config_window.rs:64
62 │ .show(ctx, |ui| {
63 │ ui.label("Editor Settings");
64 > ui.group(|ui| {
65 │ ui.label("Project name");
66 │ modified |= ui
⋮ 12 frames hidden ⋮
35: <luminol_ui::windows::config_window::Window as luminol_core::window::Window>::show::h7c5e2d5db3e9b425
at /home/lily/Git/Luminol/crates/ui/src/windows/config_window.rs:60
58 │ let mut modified = false;
59 │
60 > egui::Window::new("Project Config")
61 │ .open(open)
62 │ .show(ctx, |ui| {
36: luminol_core::window::Windows::display_without_edit::{{closure}}::h8c592b1075bf09fd
at /home/lily/Git/Luminol/crates/core/src/window.rs:91
89 │ // Pass in a bool requesting to see if the window open.
90 │ let mut open = true;
91 > window.show(ctx, &mut open, update_state);
92 │ open
93 │ })
⋮ 2 frames hidden ⋮
39: <luminol::app::App as luminol_eframe::epi::App>::update::h8008b08d0c79c7c6
at /home/lily/Git/Luminol/src/app/mod.rs:393
391 │
392 │ // Update all windows.
393 > self.windows.display_without_edit(ctx, &mut update_state);
394 │
395 │ // Handle loading and closing projects, and if applicable, show the modal asking the user
⋮ 22 frames hidden ⋮
62: luminol::main::h99bb11d9d9277c5f
at /home/lily/Git/Luminol/src/main.rs:235
233 │ };
234 │
235 > luminol_eframe::run_native(
236 │ "Luminol",
237 │ native_options,
⋮ 15 frames hidden ⋮
I did this to see this error:
diff --git a/crates/ui/src/windows/config_window.rs b/crates/ui/src/windows/config_window.rs
index 7cc2fd3..f87e15d 100644
--- a/crates/ui/src/windows/config_window.rs
+++ b/crates/ui/src/windows/config_window.rs
@@ -126,6 +126,14 @@ impl luminol_core::Window for Window {
)
.clicked();
if clicked {
+ update_state
+ .data
+ .convert_project(
+ update_state.filesystem,
+ config,
+ self.selected_data_format,
+ )
+ .unwrap(); // TODO handle
config.project.data_format = self.selected_data_format;
// TODO add conversion logic
}
|
Should be fixed now. I also changed the algorithm used by the path cache. Now it'll look for an exact match and then fall back to looking for any file extension. For example, if you search for "Data/Scripts", it'll prioritize a file exactly at the path "Data/Scripts" (case-insensitive) and then fall back to stuff like "Data/Scripts.rxdata" or "Data/Scripts.json". If you search for "Data/Scripts.rxdata" then it'll search for that path exactly before falling back to "Data/Scripts.rxdata.txt" or whatever. This allows project conversion to work properly while not breaking any other existing code as far as I can tell. |
melody-rs
requested changes
Jul 15, 2024
There was a problem hiding this comment.
Luminol doesn't panic anymore when removing a file, but doesn't seem to update the path cache when creating files either.
Using the previous diff and converting a project doesn't add any of the ron files to the path cache:
Opening a map will then proceed to panic.
The application panicked (crashed).
Message: failed to load map:
0: While getting metadata for "Data/Map266.ron" in a path cache
1: File or directory does not exist
Location:
crates/filesystem/src/path_cache.rs:391
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
⋮ 6 frames hidden ⋮
7: <luminol_filesystem::path_cache::FileSystem<F> as luminol_filesystem::FileSystem>::metadata::h532ea080f1c70367
at /home/lily/Git/Luminol/crates/filesystem/src/path_cache.rs:388
386 │ cache.regen(&self.fs, path).wrap_err_with(|| c.clone())?;
387 │
388 > let path = cache
389 │ .desensitize(path)
390 │ .ok_or(Error::NotExist)
8: <luminol_filesystem::project::FileSystem as luminol_filesystem::FileSystem>::metadata::heb18bdfc232007df
at /home/lily/Git/Luminol/crates/filesystem/src/project.rs:669
667 │ FileSystem::Unloaded => Err(Error::NotLoaded.into()),
668 │ FileSystem::HostLoaded(f) => f.metadata(path),
669 > FileSystem::Loaded { filesystem: f, .. } => f.metadata(path),
670 │ }
671 │ }
9: luminol_filesystem::FileSystem::read::h8e81da28ff5da501
at /home/lily/Git/Luminol/crates/filesystem/src/lib.rs:237
235 │ let path = path.as_ref();
236 │
237 > let mut buf = Vec::with_capacity(self.metadata(path)?.size as usize);
238 │ let mut file = self.open_file(path, OpenFlags::Read)?;
239 │ file.read_to_end(&mut buf)?;
10: luminol_core::data_cache::data_formats::Handler::read_data::ha120cecd0450285c
at /home/lily/Git/Luminol/crates/core/src/data_cache/data_formats.rs:51
49 │ .join(filename)
50 │ .with_extension(self.format.extension());
51 > let data = filesystem.read(path)?;
52 │
53 │ match self.format {
11: luminol_core::data_cache::Data::get_or_load_map::{{closure}}::{{closure}}::hf34186029d0d05e4
at /home/lily/Git/Luminol/crates/core/src/data_cache.rs:511
509 │ maps.entry(id).or_insert_with(|| {
510 │ let handler = data_formats::Handler::new(config.project.data_format);
511 > handler
512 │ .read_data(filesystem, format!("Map{id:0>3}"))
513 │ .expect("failed to load map")
12: std::collections::hash::map::Entry<K,V>::or_insert_with::h55e469d5832312fd
at /rustc/11f32b73e0dc9287e305b5b9980d24aecdc8c17f/library/std/src/collections/hash/map.rs:2664
13: luminol_core::data_cache::Data::get_or_load_map::{{closure}}::hc253a7c02751f9ca
at /home/lily/Git/Luminol/crates/core/src/data_cache.rs:509
507 │ RefMut::map(maps_ref, |maps| {
508 │ // FIXME
509 > maps.entry(id).or_insert_with(|| {
510 │ let handler = data_formats::Handler::new(config.project.data_format);
511 │ handler
⋮ 1 frame hidden ⋮
15: luminol_core::data_cache::Data::get_or_load_map::he7f40fb2abc05d22
at /home/lily/Git/Luminol/crates/core/src/data_cache.rs:507
505 │ Self::Unloaded => panic!("project not loaded"),
506 │ };
507 > RefMut::map(maps_ref, |maps| {
508 │ // FIXME
509 │ maps.entry(id).or_insert_with(|| {
16: luminol_components::map_view::MapView::new::h3982782ee1ed0331
at /home/lily/Git/Luminol/crates/components/src/map_view.rs:84
82 │ map_id: usize,
83 │ ) -> color_eyre::Result<MapView> {
84 > let map = update_state.data.get_or_load_map(
85 │ map_id,
86 │ update_state.filesystem,
17: luminol_ui::tabs::map::Tab::new::hc0e0a50736259528
at /home/lily/Git/Luminol/crates/ui/src/tabs/map/mod.rs:126
124 │ // *sigh*
125 │ // borrow checker.
126 > let view = luminol_components::MapView::new(update_state, id)?;
127 │ let tilepicker = luminol_components::Tilepicker::new(update_state, id)?;
128 │
18: <luminol_ui::windows::map_picker::Window as luminol_core::window::Window>::show::{{closure}}::{{closure}}::hd4df2fe47e3d1e30
at /home/lily/Git/Luminol/crates/ui/src/windows/map_picker.rs:143
141 │
142 │ if let Some(id) = open_map_id {
143 > match crate::tabs::map::Tab::new(id, update_state) {
144 │ Ok(tab) => update_state.edit_tabs.add_tab(tab),
145 │ Err(e) => luminol_core::error!(
⋮ 5 frames hidden ⋮
24: <luminol_ui::windows::map_picker::Window as luminol_core::window::Window>::show::{{closure}}::h0050d6e767428bf4
at /home/lily/Git/Luminol/crates/ui/src/windows/map_picker.rs:96
94 │ .open(&mut window_open)
95 │ .show(ctx, |ui| {
96 > egui::ScrollArea::both()
97 │ .id_source(
98 │ update_state
⋮ 12 frames hidden ⋮
37: <luminol_ui::windows::map_picker::Window as luminol_core::window::Window>::show::h089a677f2385ddc1
at /home/lily/Git/Luminol/crates/ui/src/windows/map_picker.rs:93
91 │ ) {
92 │ let mut window_open = true;
93 > egui::Window::new("Map Picker")
94 │ .open(&mut window_open)
95 │ .show(ctx, |ui| {
38: luminol_core::window::Windows::display_without_edit::{{closure}}::h8c592b1075bf09fd
at /home/lily/Git/Luminol/crates/core/src/window.rs:91
89 │ // Pass in a bool requesting to see if the window open.
90 │ let mut open = true;
91 > window.show(ctx, &mut open, update_state);
92 │ open
93 │ })
⋮ 2 frames hidden ⋮
41: <luminol::app::App as luminol_eframe::epi::App>::update::h8008b08d0c79c7c6
at /home/lily/Git/Luminol/src/app/mod.rs:393
391 │
392 │ // Update all windows.
393 > self.windows.display_without_edit(ctx, &mut update_state);
394 │
395 │ // Handle loading and closing projects, and if applicable, show the modal asking the user
⋮ 22 frames hidden ⋮
64: luminol::main::h99bb11d9d9277c5f
at /home/lily/Git/Luminol/src/main.rs:235
233 │ };
234 │
235 > luminol_eframe::run_native(
236 │ "Luminol",
237 │ native_options,
⋮ 15 frames hidden ⋮
Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
melody-rs
approved these changes
Jul 15, 2024
MolassesLover
pushed a commit
to MolassesLover/luminol-molasses
that referenced
this pull request
Jul 23, 2024
* Fix path cache `open_file` file creation code * Add `.desensitize` method to path cache * Fix path cache `.remove_file` and `.remove_dir` implementations * Fix another problem with `.remove_dir` * Remove the unwraps from `.remove_file` and `.remove_dir` * Change path cache desensitize algorithm * Update `.regen` and `.remove_file` for new path cache algorithm
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes the problem with the path cache described in #139. The problem was that the code the path cache uses to create new files when calling
.open_filewithOpenFlags::Createattempted to create the file at the wrong path.I also added a new method,
.desensitize, to the path cache that can be used to perform a case-insensitive search for a file with a specific file extension.Testing
Setting the script path to a nonexistent file in the config window and then saving the project shouldn't trigger an error anymore.
Checklist
cargo fmt.cargo clippy. If applicable, add:--target wasm32-unknown-unknown -Z build-std=std,panic_abortcargo build --releasetrunk build --release