search

xLightsSequencer / xLights

xLights is a sequencer for Lights. xLights has usb and E1.31 drivers. You can create sequences in this object oriented program. You can create playlists, schedule them, test your hardware, convert between different sequencers.
GNU General Public License v3.0
549 stars 208 forks source link

Check that timing track name doesnt conflict with models #2785 #4567

Closed derwin12 closed 3 months ago

derwin12 commented 4 months ago

If a timing track is named the same as a model, it is hidden. Checks were in place to catch a rename but were not done on the initial Add Timing track option. #2785

AzGilrock commented 4 months ago

Sure but you'll never be able to fix all the timing track / model naming conflicts. Timing tracks belong to the sequence so someone can easily create new models that conflict with timing track names that exist in sequences that aren't currently open. We probably should have a better way to deal with this...i.e. timing tracks and models should be able to have the same names.

derwin12 commented 4 months ago

oh for sure .. cant get them all but trying to close the one door...

AzGilrock commented 4 months ago

But I'm saying why do we want to prevent timing track and models from having the same name? We really should be able to track them using some type of unique identifier. We are doing way too many things using strings in the program.

cybercop23 commented 4 months ago

SequenceElements/AddElement is called with "model" type that then calls ElementExists. We can pass type to ElementExists and check that types also matches, thus allowing same names on different types, timingtracks, groups and models. 2 other functions will need to have the type added since they call ElementExists, but IMHo I don't think those 2 need to check at all - will be okay if we add the type, but maybe reduce a call.

cybercop23 commented 4 months ago

SequenceElements/AddElement is called with "model" type that then calls ElementExists. We can pass type to ElementExists and check that types also matches, thus allowing same names on different types, timingtracks, groups and models. 2 other functions will need to have the type added since they call ElementExists, but IMHo I don't think those 2 need to check at all - will be okay if we add the type, but maybe reduce a call.

I was wrong... as usual.. at least ViewsModelsPanel need to have the check, else all modes will still show up on the left... small change.. but adding type.. seems to work... image image

derwin12 commented 4 months ago

I believe there are a more than a few places where it assumes it is getting a sequence or a timing track when it does the lookup. As Gil mentions .. we search by name. Would take going thru with a fine tooth comb.. Allowing both is a no-going-back change. Anybody using older versions could fall victim to duplicate names. I dont think many people actually hit this scenario .. ie naming a timing track the same as a model, so not sure it is worth the efforts.

AzGilrock commented 4 months ago

Yeah I'm not saying we need to fix it to allow both right now. Just pointing out we can still end up with hidden names and it would be nice to fix it later.

cybercop23 commented 4 months ago

Please tell me what else I should look at... new seq, views changes, adding/renaming TT and adding same name models works... I can try groups.. what else should I try? YES... there may be crap out there not using this proper function, but what you found and what I looked at, they all use the ElementExists function, and that has been changed to account for type.