Page MenuHome

Correct toggle restrictions behavior
Needs RevisionPublic

Authored by John Bartle (johnbartle) on May 25 2014, 9:06 AM.

Details

Summary

Hello.

I have a mixture of novice, intermediate, and advanced programming skills and I was offered this task by Campbell Barton a couple of weeks ago to demonstrate my level of skill.

The task is titled ,"Outliner Toggle inverts (not so useful)".

Seeing as that I am not a professional, I must admit that, having attempted this tasks, I simply could not figure out the professional way of accomplishing it. I've thought of many ways of accomplishing this task, but I just don't know what is considered appropriate. So I settled for what I am now submitting to any reviewer.

I tell ya, I have become very interested in how a professional would accomplish this task. So, if anyone wants to just redo this tasks, and show me how it ought to be done OR just give me some advice, that would be great.


EDIT: Oh yeah, I forgot to give a link to the original task and it's description.
Here is the link: https://developer.blender.org/T40085

Here is the description:
If you select many objects (or other items too), and select Toggle Visibility, This will act as if you press on each items visibility (which makes some sense).

However this isn't really great behavior when there is already mixed visibility state.

It would be better if this worked like (de-select all).
•If there are any items enabled, disable all.
•if none are enabled, enable all.

This goes Toggle visibility, renderability, selectability too.

This was a user request, discussed with Brecht and we agreed this would be an improvement.

Good day to you.

Diff Detail

Event Timeline

What problem exactly is this trying to solve? Do you have any links to any previous discussions on this?

@Joshua Leung (aligorith)

I have since edited the task description above with the necessary links.

Along with the link in the description here is the original discussion that lead to the creation of this task.
http://lists.blender.org/pipermail/bf-committers/2014-May/043516.html

p.s.
Uh, I didn't mean to bother you, but I added you as a reviewer because I believe you are an overseer of the Outliner code.

John Bartle (johnbartle) updated this revision to Unknown Object (????).May 25 2014, 9:53 AM

Added more context for readability.

The way this is implemented isn't flexible.

preselected_objects_restriction_flag() only works for objects, and there are hard coded checks for object flags in outliner_do_object_operation.

Suggest to have one call to outliner_do_object_operation where the callback checks the state, then another call to outliner_do_object_operation which sets the state.

Campbell Barton (campbellbarton) requested changes to this revision.May 25 2014, 11:57 PM
John Bartle (johnbartle) updated this revision to Unknown Object (????).May 27 2014, 1:40 PM

This is a completely alternate .diff .

Hi Campbell.


@Campbell Barton (campbellbarton)
Barton wrote:

Suggest to have one call to outliner_do_object_operation where the callback checks the state, then another call to outliner_do_object_operation which sets the state.

============================================================

I'm not sure if you meant to use a global variable to pass the restriction state around OR if you meant to add the necessary arguments and parameters to the heads/prototypes of the relevant functions to pass the restriction state around.

If you meant the latter, I worked through this suggestion, and I simply could not figure out how to implement this suggestion without changing a LOT of arguments and parameters. Most of the new parameters would not get used, and serve only as clutter.

So I thought of a new implementation. I was trying to add as little code as possible and also change the existing design as little as possible. This implementation is not necessarily the most efficient, but it seems the best I can do. Unless you have suggestions.

NOTE:
object_xxx_unrestrict_cb()
object_xxx_restrict_cb()

I used the words unrestrict and restrict because these functions use OB_RESTRICT_XXX to fulfill their main purpose. Perhaps other words like enable and disable would be more appropriate, I really don't know.....

objects_exclusive_toggle_xxx_cb ()

Above I used the plural word object(s) because this function is designed to deal with a - set - of objects as opposed to just a single object.

I also use the word exclusive (as in mutually exclusive) because this func is meant to toggle a set of object's restrictions to one state or the other but not mixed -- which was the old toggle behavior.

Thanks for your time! Good day!

John Bartle (johnbartle) updated this revision to Unknown Object (????).May 29 2014, 9:18 AM

I fixed a bug.

I discovered this design doesn't work quite right for multiple scenes.
I also discovered this design doesn't work right for objects that are in multiple groups, and that the official Blender version has a related problem.

I'll look into this some other day.

John, are you still interested in working on this task/patch? Maybe if another new dev worked on it with you it might be more interesting to work on?

Thanks, JTa

This revision now requires changes to proceed.Jul 22 2014, 5:05 AM

John, are you still interested in working on this task/patch? Maybe if another new dev worked on it with you it might be more interesting to work on?
Thanks, JTa

Hello jta.

Yes, I am still interested in working on this small patch.

Right now I am stuck because I am waiting on an answer about whether to use campbellbarton's suggestion or some other implementation:

Suggest to have one call to outliner_do_object_operation where the callback checks the state, then another call to outliner_do_object_operation which sets the state.

Campbell's suggestion requires adding many new parameters, and many of those parameters will go unused. Of course, I am wondering if my current implementation is really any better.

I would also like a developer to critique my code and tell me what is inappropriate and how it could be made appropriate( e.g. what guidelines should I follow for creating names)

Lastly, I actually think it might be very beneficial to work on a project with other new devs.

Good day