Fix Save As #217

djdiskmachine · 2025-12-26T23:31:37Z

Save as now discards unsaved changes to current project Unsaved changes only written to project under new name Adds filesystem function Delete(Path&)
Adds optional argument to PersistencyService::Save(const char* name)

Save temp file
Copy temp file to new location
Delete temp file
Bump CHANGELOG

#212

Type of change

Fixes [BUG] - Using "Save Song As" saves to current as well as new project #212
Bug fix (non-breaking change which fixes an issue)

Test Configuration:

Tested on x64, creates a new folder containing project with unsaved changes

Checklist:

I have performed a self-review of my code
I have commented particularly in hard-to-understand areas
I have updated CHANGELOG
I have updated docs/wiki/What-is-LittlePiggyTracker.md reflecting my changes
I have version bumped in sources/Application/Model/Project.h
My changes generate no new warnings (build without your change then apply your change to check this)

Save as now discards unsaved changes to current project Unsaved changes only written to project under new name Adds filesystem function Delete(Path&) Adds optional argument to PersistencyService::Save(const char* name) Save temp file Copy temp file to new location Delete temp file Bump CHANGELOG #212

danpicton

Not sure if this is helpful, so feel free to ignore!

danpicton · 2025-12-28T09:06:16Z

sources/Application/Views/ProjectView.cpp


-		Path path_srclgptdatsav = path_srcprjdir.GetPath() + "lgptsav.dat";
-		Path path_dstlgptdatsav = path_dstprjdir.GetPath() + "/lgptsav.dat";
+        Path path_srclgptdatsav = path_srcprjdir.GetPath() + "lgptsav_tmp.dat";


I can see this is similar to what was there previously, but are we good with the inconsistency between the two filenames (i.e. one is prefixed with /, the other isn't)?

Yes this is required as the srcprj variable is fetched using project: which returns a trailing front slash :)

Cool, thanks for clarifying :)

danpicton · 2025-12-28T10:13:43Z

@djdiskmachine - I've built for x64. When I try and "Save Song As", then when selecting a random name or creating a new name, LGPT crashes with this error:

/usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:1369: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::reference std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator[](size_type) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; reference = char&; size_type = long unsigned int]: Assertion '__pos <= size()' failed.
zsh: IOT instruction (core dumped)  ./lgpt.x64 -SCREENMULT=4

The save is unsuccessful.

Standard "Save Song" functionality is unaffected.

djdiskmachine · 2025-12-28T10:43:04Z

How strange I've only tried it in the psp emulator, I'll have another look at it then :)

djdiskmachine · 2025-12-28T14:28:34Z

I made a test build yesterday, you can download these here

save_as_fixed.mov

djdiskmachine · 2025-12-28T15:14:29Z

.. And for the build created by this latest check run

save_as_fixed-1-6-0-bacon1.mov

djdiskmachine · 2025-12-28T17:08:42Z

So i've tried on my ubuntu machine too built on this branch, can't recreate the issue

Can you recreate it again then send me the lgpt.log file?
It's cleaned out on each boot so you can't send an old one.

danpicton · 2025-12-28T20:29:40Z

Hmm - strange. Just tried your test build and getting exactly the same issue.

I've got AMD Ryzen 7 5700G CPU running Arch with kernel 6.17.6-arch1-1.

I've tested the RG35XXPLUS build and that's working fine 🤷‍♂️

Please let me know if there's any other info that may be helpful!

danpicton · 2025-12-28T20:55:10Z

lgpt.log

I couldn't find lgpt.log, so enabled log events and redirected to file - is that what you wanted?

djdiskmachine · 2025-12-29T07:53:26Z

debug.patch
Yes that works, usually you set DUMPEVENT in the config.xml

OK I can see it crashes immediately when you press your A button (Z) on the keyboard
No hints in the log files as to why.

Probably need to add more logging in the troublesome parts to figure out why. Can you run with the attached patch file applied?

Looks like you have some special kind of setup for your config, i.e. songs and samplelib in another location than your lgpt binary?
Is the same problem there if you have a standard setup (bin/, samplelib/ lgpt_songFiles all in one folder?)

danpicton · 2025-12-29T11:57:53Z

Here's an lgpt.log of the behaviour after applying the patch.

Here're my config.xml and mapping.xml. The failure persists even if I remove the custom ROOTFOLDER and SAMPLELIB config.

danpicton · 2025-12-30T17:07:05Z

@djdiskmachine I've been doing some more debugging, and this seems to do the job for my x64 build:
possible_x64_fix.patch

I used Gemini, but the change makes sense. Possibly it is something to do with my non-default directory setup, even though my testing didn't surface anything. I don't think it would, but hopefully it doesn't break any of the other builds 🤞

djdiskmachine · 2025-12-30T19:14:55Z

Interesting. So this Is caused by an uninitialized path string then. With this fix, if the path is null we don't set it to the root of the file system which is probably a good guard.. but what does the "" path become then?

djdiskmachine · 2025-12-30T21:47:25Z

Seems like I caused a regression with this PR #215
The optional Path argument here should be "root:" not an empty string
NewProjectDialog(View &view, Path currentPath = "");

Your fix is good for other stuff tho.
If one was to be in
/path/to/leaf

and currentPath for some unexpected reason becomes ""
the Descend(leaf) would do

cd ./leaf

which is a whole lot less dangerous than allowing

cd /leaf

Thanks a lot for all the help I'll add both these fixes :)

Call ACTION_SAVE_AS NewProjectDialog with "root:" as path Change default currentPath to "root:" (fix regression caused in #215 Disallow accidental root folder access in FileSystem::Descend

danpicton · 2025-12-30T22:12:40Z

yw man - happy to help. Thank you, too! 🙌

djdiskmachine · 2025-12-30T22:15:47Z

lgpt.x64.txt
Here's a test build if you'd like to help me verify.
I still can't reproduce, I guess my OS doesn't allow willy nilly access to folders at the top of the file system =D

danpicton · 2025-12-31T09:08:53Z

That seem to have done the trick! 🎉

danpicton

Looks good ✔️

djdiskmachine added 2 commits December 26, 2025 23:25

Initialize path as std:string instead of auto

1c1c362

danpicton reviewed Dec 28, 2025

View reviewed changes

djdiskmachine added 2 commits December 30, 2025 22:02

Guard against forbidden paths

68d1997

Call ACTION_SAVE_AS NewProjectDialog with "root:" as path Change default currentPath to "root:" (fix regression caused in #215 Disallow accidental root folder access in FileSystem::Descend

Actually change the default optional arg to "root:" in NewProjectDialog

0c7b1d1

danpicton approved these changes Dec 31, 2025

View reviewed changes

djdiskmachine merged commit 407fe8f into master Dec 31, 2025
12 checks passed

djdiskmachine mentioned this pull request Jan 3, 2026

[BUG] - uninitialized currentPath in Filesystem can enter folders in file system top #219

Closed

Fix Save As #217

Fix Save As #217

Uh oh!

Conversation

djdiskmachine commented Dec 26, 2025

Type of change

Checklist:

Uh oh!

danpicton left a comment

Choose a reason for hiding this comment

Uh oh!

danpicton Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

djdiskmachine Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

danpicton Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

danpicton commented Dec 28, 2025

Uh oh!

djdiskmachine commented Dec 28, 2025

Uh oh!

djdiskmachine commented Dec 28, 2025

Uh oh!

djdiskmachine commented Dec 28, 2025

Uh oh!

djdiskmachine commented Dec 28, 2025

Uh oh!

danpicton commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danpicton commented Dec 28, 2025

Uh oh!

djdiskmachine commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danpicton commented Dec 29, 2025

Uh oh!

danpicton commented Dec 30, 2025

Uh oh!

djdiskmachine commented Dec 30, 2025

Uh oh!

djdiskmachine commented Dec 30, 2025

Uh oh!

danpicton commented Dec 30, 2025

Uh oh!

djdiskmachine commented Dec 30, 2025

Uh oh!

danpicton commented Dec 31, 2025

Uh oh!

danpicton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

danpicton commented Dec 28, 2025 •

edited

Loading

djdiskmachine commented Dec 29, 2025 •

edited

Loading