lsdsng-import should ignore .WM.lsdsng in filename

urbster1 commented 5 years ago

if more than 1 lsdsng is imported, just put the first one in working memory. as-is, WM files don't append to the save file and if more than 1 WM lsdsng is imported, one of them gets lost.

stijnfrishert commented 5 years ago

What do you mean with "don't append"?
If more than one WM lsdsng is imported I'd argue you're doing something weird in the first place :p

But yeah, makes sense to just select the first lsdsng as WM. Why just with more than 1 though?

urbster1 commented 5 years ago

I had 2 saves and I wanted to merge them together and use the songs in working memory from both, that's why. Found out that one of them got placed into working memory only, but not the save itself, and the other one didn't add at all. Maybe it's worth checking to see if 2 song names are identical but one has WM and the other doesn't?

On Wed, Jul 10, 2019 at 6:04 AM Stijn Frishert notifications@github.com wrote:

What do you mean with "don't append"?

If more than one WM lsdsng is imported I'd argue you're doing something weird in the first place :p

But yeah, makes sense to just select the first lsdsng as WM. Why just with more than 1 though?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stijnfrishert/liblsdj/issues/56?email_source=notifications&email_token=ABLMCWOONAUHLR3R4GJNPHTP6WX2HA5CNFSM4H3XV2BKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZS7MPA#issuecomment-509998652, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLMCWP7D2OL5KX46KYPMYLP6WX2HANCNFSM4H3XV2BA .

rbong commented 4 years ago

It seems like this is another issue I can handle

stijnfrishert commented 4 years ago

Sure, I'd say go ahead! Are we all agreeing that it's a good thing the import tool should ignore the working memory song (@rbong I'm assuming you've worked with the tools and lsdj before)?

(fyi, not related to this issue: I'm currently reworking the library code for a 2.0.0 with some big overhauls, but any issue that has to do with the tools' code is safe to work on!)

urbster1 commented 4 years ago

if you're asking me, i'd prefer for anything with .wm in the filename be ignored and for the first song in the input to go into working mem. but maybe a flag makes more sense. the main issue here i think was that if you have two songs that are .wm songs, one of them gets ignored, so at least that behavior should probably be changed

On Sun, Feb 9, 2020, 5:50 PM Stijn Frishert notifications@github.com wrote:

Sure, I'd say go ahead! Are we all agreeing that it's a good thing the import tool should ignore the working memory song (@rbong https://github.com/rbong I'm assuming you've worked with the tools and lsdj before)?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stijnfrishert/liblsdj/issues/56?email_source=notifications&email_token=ABLMCWKKKPFI6W45DDGMS6LRCCCCVA5CNFSM4H3XV2BKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELG2TNA#issuecomment-583903668, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLMCWOKXK2GQ2X7IPIQNFLRCCCCVANCNFSM4H3XV2BA .

rbong commented 4 years ago

I've been thinking more about this issue and I think this would be the most flexible behaviour:

Provide a flag to explicitly put a song into working memory
Ignore the .WM extension and treat these like regular .lsdsng files
Do not put a song into working memory otherwise

Tried to think of something we could do to guess what the user is trying to do but I don't think you can account for every case without explicit flags.

rbong commented 4 years ago

Just realized that we might have to guess/provide some options for working with directories. If the user specifies lsdj-import -o lsdj.sav --working-file folder/file.WM.lsdsng folder/ and we're treating the WM file as a regular file, the WM file will be placed in its own save slot as well as working memory, which may or may not be the user's intention. Open to solutions.

stijnfrishert commented 4 years ago

Right, I fully agree with your original original idea; just add a flag to explicitly set a working memory song and otherwise simply don't. Guessing for the user always ends up generating problems in edge cases.

If a folder contains a .WM it would be put in its own slot, at which point I think we shouldn't export it on default in lsdsng-export (or at the very least don't if the file_changed flag isn't set).

In regards to your second comment: why don't we simply check for duplicates?

stijnfrishert commented 4 years ago

(Accidentally closed)

rbong commented 4 years ago

In regards to your second comment: why don't we simply check for duplicates?

That's a good idea. To expand on this idea a little bit, I think this behaviour would make sense when importing folders:

Check folder for names in the form *.wm.lsdsng
When one is found, if a working song wasn't specified, no working song was already found, and it's not a duplicate of any file in the folder, use it as the working memory song
Otherwise, skip *.wm.lsdsng files in folders and print that we are skipping them as well as whether they are duplicates

There is a bunch of other prioritization we can potentially do but it's better if we rely on making the user specify the working memory file or just rely on them putting the appropriate folder first. We can potentially add flags for changing this behaviour in the future but I think users will usually want to skip most *.wm.lsdsng files when importing multiple folders, or even multiple *.wm.lsdsng files in a single folder.

Let me know what you think.

stijnfrishert commented 4 years ago

Completely agree on this. And if we (good suggestion) just print out we skipped a couple of files, they'll know and it's up to users to select which WM song they do want with the flag.

In any case, I say go ahead! :)

stijnfrishert commented 4 years ago

@rbong: I'm slowly nearing a v2.0.0 release (gotta rethink the error API and waiting for some test files from @urbster1).

Do you want this issue to get in v2.0.0 as well (I'm assuming you're working on this)?

rbong commented 4 years ago

@stijnfrishert If you can wait a couple of weeks, this can go in v2.0.0

stijnfrishert commented 4 years ago

Hm, I hope to release sooner than that, but then we'll just add this to v2.1.0. :)

No rush either way, just wanted to know what milestone to put this under.

stijnfrishert commented 4 years ago

@rbong Any news on this?

If you can't find the time right now that's fine, in which case I might tackle this for the upcoming release (I've found some spare time/motivation lying around :p)

rbong commented 4 years ago

No progress on this, sorry. This fell off my radar.

stijnfrishert commented 4 years ago

No problem, thanks for letting me know :)

stijnfrishert commented 4 years ago

Importer now has a --working-memory flag to provide the working memory song.
All .WM files are ignored
- Only in a folder. When you provide it manually it's just another project slot

Does that work for you, @urbster1?

urbster1 commented 4 years ago

Sure

On Tue, Nov 10, 2020 at 11:54 AM Stijn Frishert notifications@github.com wrote:

Closed #56 https://github.com/stijnfrishert/libLSDJ/issues/56.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stijnfrishert/libLSDJ/issues/56#event-3979948353, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLMCWPT5I34AYAH7FXAL4TSPFV5XANCNFSM4H3XV2BA .

stijnfrishert / libLSDJ

lsdsng-import should ignore .WM.lsdsng in filename #56