Skip to content
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.

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

Bugfix: Item Saving #7

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

Conversation

SteeveMonkey
Copy link

@SteeveMonkey SteeveMonkey commented Jul 6, 2024

Changes items handlers to use record IDs as the names of files instead of the item name. This fixes items with identical names from being overwritten on save (Issue #2) and allows items with special characters in its name to be saved.

Retains the ability to delete records named by their associated item's name as a fallback if it is not named by record ID.

However, naming causes the order of items to be fairly random rather than following any sensible sorting methods.
Ideally, this would be changed so that they are sorted by date to match how the normal cloud inventory works or possibly even match the selected sorting option from Better Inventory Browser if it is installed

Items and links now use their corresponding record ID as the name for
their locally stored files instead of the name of the item or link.
This fixes the issue with items being overwritten due to the name being
the same, since record ID's are more unique than item names. This also
fixes and issue where items with certain special characters in their
names could not be saved.

Fixes: art0007i#2
Allows deletion of records that use an item name, as well as records
that are named by record ID
Thumbnails stored in the same place as it's corresponding item will now
be deleted with that item
@SteeveMonkey SteeveMonkey marked this pull request as ready for review July 6, 2024 19:12
@art0007i
Copy link
Owner

art0007i commented Jul 7, 2024

the thing is I actually use the names to find these files. if they were random ids then I can't use that anymore. Ideal solution would be to add a suffix/prefix like item.json then item-1.json item-2.json etc.

@SteeveMonkey
Copy link
Author

Yeah, that is the other big issue with this solution, that I now realize I had forgotten to mention.

If the ability to tell what an item is by file name alone is important, then I agree that using a prefix or suffix in the file name would probably be best.
I think using a timestamp as the prefix would be a good way to have a unique prefix while simultaneously causing them to be sorted by date, matching the cloud inventory. Though, this could clutter up the beginning of the file name a bit.

Now, I would like to retain the ability to save items regardless of what the item name is, that is a pretty big one for me. Currently, they just don't save if anything about the name is invalid for a file name. So, the names will need to be trimmed of any invalid characters and possibly cut off after a certain length to be used as a file name.
And if the file name should reflect the item name and human readability is a concern, then I think a good approach would be to trim out all the formatting tags and replace any remaining invalid characters with, probably, an underscore.

With all of that covered, making any of these changes to the file names would make it harder to reference the record files later in order to delete them. Since the item delete function seems to only have access to the contents of the record file and does not have a direct reference to it, the filename will either have to be possible to rebuild from the information in the record, or just be included in the record in the first place.
Either that or find another way to get the path for the record file. It's clearly being accessed at some point to get the contents, so maybe finding a way to pass the file reference along to the delete function would be a good way to go. This would have the added benefit of allowing the file to be renamed to almost anything, and retain the ability to delete it in-game.

@SteeveMonkey
Copy link
Author

Another solution would be to make a small application or script to help with managing LocalStorage items without needing to run Resonite. Would basically be a file browser that is purposefully built to manage the local Inventory. This could include features to open files in a text editor or highlight the file for you in your file browser to assist with manual file management. It could also double as an automatic item migration tool to rename older items to the newer naming scheme. And in the future, it could be expanded upon to allow management of the cloud inventory once it is possible to do so via API calls.

This is something I have already considered doing, but it would of course take a while to make. However, if you are fine with using something like this and keeping the record IDs as filenames, then you could just wait to merge until I have something you feel you could work with.
But, if you would really like to retain the ability to manage the LocalStorage files completely from your own file browser and text editor without the help of another application, then I am willing to try and implement a naming scheme that you can agree with, just let me know what you think of my suggestions for a new one so far

@art0007i
Copy link
Owner

art0007i commented Jul 9, 2024

I looked around to see if it's possible to keep a reference to the file path in the record and I think I found one. the record has a description field which seems completely unused by the game so it would be easily possible to store file paths there.

Important thing is to do this when loading the file and not when saving it because a user might move the files manually https://github.com/art0007i/LocalStorage/blob/master/LocalStorage/LocalStorage.cs#L221
In this line instead of record.Description it could have the record's filepath and then in the delete function read the description to find the correct path.

Then for avoiding invalid names like you said: remove rtf tags then change remaining chars into a fallback char (or just remove them idk what looks better).

For collisions I'd say best option would be like I initially said, just add a suffix until the file doesn't repeat so "item.json" then "item-1.json" then "item-2.json" etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants