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

Prevent layers to have the same name #1086

Closed
davidlamhauge opened this Issue Oct 3, 2018 · 12 comments

Comments

Projects
None yet
5 participants
@davidlamhauge
Copy link
Contributor

davidlamhauge commented Oct 3, 2018

Issue Summary

It is possible to have two or more layers with the same name

Actual Results

  • I have a bitmap layer called "Bitmap Layer"
  • I press the "New Layer"-button, and choose "New Bitmap Layer"
  • I name it "Bitmap Layer"
  • I now have two layers called "Bitmap Layer"

Expected Results

You shouldn't be able to have layers with the same name

Video or Image Reference

No

Steps to reproduce

As described above

System Information

  • Pencil2D Version:
    0.6.2
  • Operating System:
    Linux, Mac (And probably Windows too)
  • RAM Size:
  • Graphics Tablet:
@Jose-Moreno

This comment has been minimized.

Copy link
Member

Jose-Moreno commented Oct 3, 2018

I completely agree that the software generated names for layers shouldn't be the same when creating new resources e.g if I create a new bitmap layer, it should follow a numerical sequence bitmap 1, 2, 3 etc,

To avoid having problems with duplicate names though... some programs deal with this by giving the layer a "unique" name or identifier and a "nameable" display (pretty much like a forum username vs display name) I believe Pencil2D has a unique numerical index for each layer, that's why we can name them the same without (apparent) technical issues.

Note: On the other hand since we're discussing this topic, I'd like to leave my opinion on a related consideration, where user generated names (the ones written by user input) should be allowed to have identical strings. Be it by mistake or convenience, prompting an error on identical strings would feel like "punishing" the user when naming two layers the same just because, and I think it should not be considered in any event. If possible I'd others to provide input on this consideration as well if that's ok. 🙂

@davidlamhauge

This comment has been minimized.

Copy link
Contributor Author

davidlamhauge commented Oct 3, 2018

I worked 20 years as a professional animator, and I have never, ever worked on a scene with identical layer names. The danish convention is to use 'A', 'B', 'C' and so on, but it could be 'Body', 'Head', 'Mouth' and so on. Who would ever want four layers called 'Joe', 'Joe', 'Joe' and 'Joe'?
I don't think we should suggest names. We should give the user an empty frame, and let him pick an appropriate name, and if he picked a name already in use, then he would get no new layer OR a layer with his suggestion + '1', so he had 'Joe' and 'Joe1'.

@Jose-Moreno

This comment has been minimized.

Copy link
Member

Jose-Moreno commented Oct 3, 2018

@davidlamhauge You're right in the sense that we should let the user pick the names. I too have used the A,B,C convention and thus I never name my levels / layers the same, not even by chance.

What I was suggesting was renaming them sequentially as you mentioned in your "Joe+1 = Joe1" example.

Another consideration to help eliminate this problem from root which I mentioned on discord, is that we should remove the naming dialog that is brought on layer creation. Right now Pencil2D forces you to name each one of them individually, and if you rapidly press enter you end up with the same name, which is the actual problem.

If I want 10 layers I want them to be created quickly, and I can name them after they are created, and as such adding sequential numbers to the base name (even if it's an empty string) would help differentiate the layers AND solve the issue.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Oct 3, 2018

Even if it doesn't make sense to have 4 layers named "Joe" I don't like that we try to enforce that on the user, again by nagging. If it's to avoid duplicates in some other scenario, for example project structure or (by reading discord) the copy frame dialog feature, then i'd say there are better ways, namely using unique identifiers instead. What we could do however is to name the layers incrementally instead of naming them simply "Bitmap Layer" or "Vector layer"

it's true that if the user chooses to name their layer "Joe" twice and wants to use the duplicate, reverse keyframe feature, then that's a fault of the user and I don't think the application should "fix" that.
No drawing, animation application that I know of enforces this, so why should we need/require it?

@scribblemaniac

This comment has been minimized.

Copy link
Member

scribblemaniac commented Oct 4, 2018

prompting an error on identical strings would feel like "punishing" the user when naming two layers the same just because.

This really sums up my opinion of this topic well. It's not a bug that layers can be named the same, it's a feature. No I can't think of any good scenario where users should keep multiple layers named the exact same, but enforcing that they be unique is an arbitrary restriction for the user. There is no reason technically why we can't support multiple layers with the same name, all the code should use the ids internally. If users have trouble choosing from a dropdown menu, then they will immediately realize that they need to rename their layers when there are 4 Joes in the list.

For one practical use of non-unique names, imagine you have two layers, let's say bob and fred. But after doing your work, you realize you drew bob on fred's layer and fred on bob's layer. The solution is pretty simple, switch the layer names. Right now you can change bob to fred, and then fred to bob. If unique names are enforced, you have to change bob a temporary value before renaming the other to bob, and then rename that temporary value to fred. It's very unintuitive and could be frustrating for a user who doesn't understand this mechanic very well.

I agree that creating unique names by default with some numbering scheme is something useful that should be implemented. I also think that automatically naming without the popup as @Jose-Moreno is a good idea as it's less disruptive to the workflow, but those are both separate issues.

At least to me, this seems to also be what @Jose-Moreno and @CandyFace are saying, so I think that #1087 should be reverted, especially if there's going to be a patch release soon, until at least we come to a consensus on this matter.

@davidlamhauge

This comment has been minimized.

Copy link
Contributor Author

davidlamhauge commented Oct 4, 2018

My patch pervented the user from giving identical layer names 'at birth'. I don't think it prevents the user to rename the layer later to a name that exists. It should be possible to change ''bob' to 'fred', even if 'fred' exists. I'm at work and can't test it now.
I think #1087 should stay, but it is up to you.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Oct 4, 2018

There are less obstructive ways to prevent users from using identical names, by for example incrementing the layer names instead. If we require that layers should have unique names at birth, then that rule should be consistent. Otherwise that could be considered a bug.

@Jose-Moreno

This comment has been minimized.

Copy link
Member

Jose-Moreno commented Oct 4, 2018

@scribblemaniac Indeed removing the naming dialog for new layers and the auto-name numbering are separate issues, but those are still related to this because the root cause of this problem is the name dialog pop-up the same name every time you make a new layer.

I honestly think it should be easier to test this change and get a feel for what's happening, too many what's and if's on the theoretical side of things isn't getting us anywhere. I'm positive it's a simpler fix and behavior than what is being presented.

So if the layer-same-name fix scope is applied only on layer creation, then that should solve the problem at hand, and preserve the cited requirements to allow users to change the name to whatever they want afterwards. It is possible the auto-numbering isn't even needed, but perhaps the dialog removal is, which have to be discussed separately.

Either way I'd like to avoid more potential oversights so I'll gladly sit down and make a full test on every element of the software tomorrow (it's 1am here) to at least be positive we didn't miss anything this time around. I'll try to update the source code and build it on QT so I can provide assistance on testing this issue as well until there's a nightlybuild available after @chchwy patch.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Oct 4, 2018

@Jose-Moreno why do you think those are separate issues? if there's no dialog and the layer is added with increments, then the PR that was pushed over night would have no meaning, since it only triggers at birth. The PR was made to "fix" duplicate names in the dialog but that would never happen since there would be no dialog.

So if the layer-same-name fix scope is applied only on layer creation, then that should solve the problem at hand, and preserve the cited requirements to allow users to change the name to whatever they want afterwards. It is possible the auto-numbering isn't even needed, but perhaps the dialog removal is, which have to be discussed separately.

The problem with this is that we apply a rule that is simply arbitrary in a special context and has no meaning otherwise, you say to the user that the name should be unique but we don't enforce that rule ourselves later, which just seems odd to me. Either do or don't, let's not apply half rules.

I've said it before and I'll say it again: Popups that are not explicit opened are obstructive and annoying and should be avoided at all costs, they're last resorts and should only be used when absolutely necessary.

@davidlamhauge

This comment has been minimized.

Copy link
Contributor Author

davidlamhauge commented Oct 4, 2018

I've said it before and I'll say it again: Popups that are not explicit opened are obstructive and annoying and should be avoided at all costs, they're last resorts and should only be used when absolutely necessary. >

I disagree. The popup that urges you to save your file, while you are animating is obstructive and annoying, but in many instances popups are informative, and they urge the user to adapt a behavior that direct to best practices. It's then up to you, if you want to adapt or go your own way.
That said, I've come to think of a practical use of identical layer names. If you have a bitmap layer 'bob', and want to trace it to a vector layer, then it would be logical to name this layer 'bob' as well.
Maybe we should leave as it was, and then in our documentation and tutorials, recommend that each layer should be given meaningful and distinct names.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Oct 4, 2018

The popup that urges you to save your file, while you are animating is obstructive and annoying, but in many instances popups are informative, and they urge the user to adapt a behavior that direct to best practices. It's then up to you

I totally agree that the save your file popup is annoying but I also voted against that. it's one we have though to avoid users hoping that their work was saved even though they never explicitly did it themselves.

Popup dialogs that force a user interaction are not meant to be informative, that's what tips and documentation is for, this kind of popup takes the users eyes away from the content and forces them to click and accept before they can continue. Best practice should be encouraged, not forced.

anyway, your practical use of identical layers for vector and bitmap too verifies that having this arbitrary rule is not a good way to go about it.

@Jose-Moreno

This comment has been minimized.

Copy link
Member

Jose-Moreno commented Oct 12, 2018

The behavior for this has been discussed and agreed to outside of this thread. Now new layers are created with an automated number appended to the generate name for the layer e.g Bitmap Layer 1, Bitmap Layer 2, etc. This way there will never be a newly created layer that has the same name. However you retain the ability to change the name and rename each layer to the same name if you want. This was implemented in #1091

Closing.

@chchwy chchwy added this to the 0.6.3 milestone Oct 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.