Script editor window closing loses already saved changes about artofillusion HOT 21 OPEN

Ouch. We should get this sorted out.

Question: How do you end up with multiple script editor windows in the first place? Should this even be possible?

from artofillusion.

How do you end up with multiple script editor windows in the first place? Should this even be possible?

It happens the same way that it would happen with any AoI editor. You can end up having several editors open with the same object in them not realizing that yo do. The latest one you save the object with, will overwrite everything that was done or saved before.

At the very least WindowClosinEvent shoud be routed to cancel and never ever in any imaginable case directly to "OK" or "save". Cancel may or may not ask if the changes made should be saved or not but the the common expectation is that WindowClosing just closes the window and makes no changes into anything at all.

The thing that by double clicking any object you can open it in as many editors as you like without the editors (or objects in them) being in any way aware of each others is an AoI wide problem. The usual practice in the rest of the world is that if you try to open an object that is already open in an editor, the existing editor is brought to front an activated and you simply cannot have several editors working on a (different copy of) the same object.

from artofillusion.

It happens the same way that it would happen with any AoI editor. You can end up having several editors open with the same object in them not realizing that yo do.

This actually seems to be the core of the problem. I thought that this was supposed to be impossible, but it seems not.

I'd really like to have some consistency here. Either:

• All ObjectEditorWindow variants map WindowClosingEvent to dispose, (And I think that this has been discussed in the past, to much push-back from the user forum, as it makes it easy to accidentally drop your changes)
• Cannot open a second editor window for an object. Fixes you problem and still keeps the fail-safe. This would probably look like the "edit object" code searching for an already open window, and bringing it to the front if it exists.

from artofillusion.

Edited form the e-mail version. :)

map WindowClosingEvent to dispose,

Not always diresctly to dispose. A pretty common practice is to route the the red X to the same path with Cancel, which may or may not ask, whether the changes sould be saved or not. Usually there is a reason behind the implementation of cancel. For example:

• On mesh editors it is a good idea to rig cancel to ask if the changes should be discarded and if "no", return to editing.
• On primitives cancel probably could just discard and dispose
• On tools that don't edit objects but rather create somethig new form existing objects and/or input values the logical approach IMO would be to have Cancel keep the changed settings for later use, but not complete the job and red X to discard changes and dispose.

In the case of the ScripdedObjecEditor Cancel calls dispose directly.

Cannot open a second editor window for an object. Fixes you problem and still keeps the fail-safe. This would probably look like the "edit object" code searching for an already open window, and bringing it to the front if it exists.

Exactly. If my memory serves me right, AoI already keeps some kind of a list of what windows are open, but that probably does not carry enough information for this purpose. ... For one thing ALL windows and dialogs should be added to that list at open and removed at close and there needs to be the information available what each "tag" is working on. I'm not sure if it can be found trough every dialog or window or if it should be marked on the list?

Some pseudo code...

ArrayList<Object[]> openEditors = new ArrayList<Object[]>;

add(Object editor, Object editee)
{
    OpenEditors.add(new Object[]{editor, editee});
}

remove(Object editor)
{
    for ...
}

Object ifOpen(editee)
{
    for(Object[] oe : openEditors)
        if (oe[1] == editee)
            return oe[0];
            // bring oe[0] to front and activate
        else
            return null;
            // launch a new editor
}

Probably not exactly correct and there might be othere ways of doing it (try-block?) but that's the principle how I'd do it. --And I think editors (at least some of them) actually carry a new copy of an object while editing, so the reference on the list need to be to the original one.

And yet there is the problem that some (at least matrial and texture) editors are modal dialogs and some are not modal. I don't think that any of those really should be modal: It should be prossible to have a several open (to different textures of course) so you could copy values dialog to dialog, like from a uniform texture to a procedural one -- not such an unusual operation that one.

EDIT: A change to the code model. Looks minor but makes great difference.

from artofillusion.

editors (at least some of them) actually carry a new copy of an object while editing

And this would be the "tricky" bit. I used to think that all of the Object editors were modal, and actually that might be the simplest way to go, despite the inability to copy between editors that would enforce.

from artofillusion.

Did a bit of prototyping. So:

• The editor list always has a reference to the original object, so it does not matter whether there is the original or a copy on the editor.
• The editor should remove itself from the list at dispose. Otherwise it will not be truly disposed.
• Unfortunately this will have to work from bottom up. --> A lot of files to edit. Relatively simple though, so I'd say probably worth it.
• I wonder if it were possible to have an approach that always keeps child-windows front of the parent? Maybe not mesh editors but most of the others. I thing BDialog should do that, BFrame not.

I'll PR the prototype.

EDT: BTW, one very typical way to open several editors is to accidentally click 3 times to edit: The first two will open the editor and the third bring the parent window on top --> Looks like nothing happened and you double click again....

from artofillusion.

I thought that I would be able to do a fairly non-intrusive fix for this issue. Unfortunately, it depended on editing windows being added to the open windows list, and that is not currently happening. I'd like for this behavior to be hard-coded in so that plugins pick it up automatically. (Is there any reason that a plugin would want to not be limited to a single editor window per object?)

That would require some breaking changes to the API.

from artofillusion.

Is there any reason that a plugin would want to not be limited to a single editor window per object?

Not really but.... (Very OOT) This is actually a normal situation in engineerin modelling, when we talk about 'concurrent engineering' and of course that would not apply to plugins only but everything. The catch is that only one of the editor windows can save changes to the objects:

• You can open an object (a part or an assemby) in editing mode or viewing mode. You can make changes in viewing mode too, but they will not be saved unless you switch to editing mode before saving. Naturally you can not switch to editing mode if someone else already has the object opened for editing.
• Still in one session you can only open one editor per object.
• When a team is workin on a large assembly, each viewed copy of the assembly can be updated with the changing parts. Then eventually somebody will have to claim editing rights and save it with the latest versions of parts.

Probably not a very typical way of using AoI ever. One potetntial problem in concurrent modelling is application crashes... The editing mode may be left locked and then somebpdy will have to have rights to unlock the locked object or 'steal' the editing right.

from artofillusion.

That's... Interesting. Sort of a "check out object for modification" system. I'd guess that these are all different running instances of the software? That would have to be more of a "lock file for editing" situation. Not at all relevant to what we're working on here... It would have to be a separate layer. Maybe doable as a plugin, and still better off if we can fix this.

from artofillusion.

I'd guess that these are all different running instances of the software? That would have to be more of a "lock file for editing" situation.

They are very complex systems and some work better than others. Some lock the CAD entirely into a database and some allow working more freely and using the product data management when you actually have a product. Product codes, verision control, variant control, drawing numbers, approval cycles, material consumption forecasts....

"check out object for modification"

Exactly. Some say 'check out' some 'open to edit' :)

from artofillusion.

Question: How important is it to be able to have more than one EditingWindow (for different objects, of course!) open at a time? (ie, two different TriangleMeshes) I think the actual fix is going to be a bit deeper than it seemed at first.

from artofillusion.

How important is it to be able to have more than one EditingWindow (for different objects, of course!) open at a time?

Having several objects open at the same time is possible in the rest of the 3D modelling world. I'd say that it's a very basic functionality that anybody with any modelling experience expects to be possible. If that all of a sudden would no longer be possible I'd be pretty serious step backwards.

It's a different issue that some windows (like TriangleMesh) don't (for now) have tools to copy stuff from one object to another, but for example script and texture editors do and even for those that don't, it needs to be possible to at least check e.g. coordinate values in one object and use them on an another one without having to open one, close one, open other, save other, close other, open one...

from artofillusion.

Hey, my two cents : I think the general issue is not having multiple editors open on a single object, it's having multiple "save" buttons enabled for the same object.
Example use case : one could want to keep a window open for a given script as a reference, and then work on another one. The first opened window (the one being worked upon) would have an active "save" button, and the other would have a greyed out and disabled one.
We can generalize to editor windows applying to other object types - say, meshes. This would both allow multiple editors open, thus enabling copy/pasting as peteihis mentioned, and remove any ambiguity about which editor can actually update the object.

from artofillusion.

Hmm... I'd think that you'd want to disable more than the save button. Pretty much all of the editing tools, etc. should not function at all in such a scenario (Otherwise , you'll end up putting in a lot of work before realizing that you've been editing in the wrong window.)

How would you see this working on a code level? I suspect that enforcing a single window per object will be just as simple, if not simpler. If you really need a copy of the same object for reference, do just that: make a copy, and then open them side by side.

from artofillusion.

(the one being worked upon) would have an active "save" button, and the other would have a greyed out and disabled one.

This thinking would be too confusing in many ways. How this is done is that you simply make another copy of the object you are going edit. Then you will have the pre-edit version safe in case you wish to go back. In the case of scripts you can open a new empty script and copy the existing script on that one, before you start to work on it.

My current daily tool has options for save, like the usual "Save", then "Save as...", Save a copy", "Save a copy and open". How those appear, depends on the case but generally speaking the last three are found under "Save as" either on the dialog or as a pop-up selection.

This seems to be a perfect case to apply the KISS-principle. :D (To me it was introduced as "Keep It Simple (and) Stupid", where you could replace the and by a comma depending on the case....) This is definitely an issue that has been dealt with in all editing software since the beginning of time, so it probably is safe to say that all the ideas have already been been tested and the good ones have survived.

On more general level, I have seen a lot of usability downgrades happening over the years as new developers either don't know exiting practices or start to over think their case. Usually the basic principles that have been adopted in the very early years have been very well premeditated and tested and trying to improve those usually really does not.

from artofillusion.

As I was working on tool scripts (not object scripts, but I think the editing problems are the same - correct me if I'm wrong please, and tell me if this post is at the wrong place), I lost my work several times. The causes were :

• not realizing I had more than one window opened on the same script, and overwriting the script with "old" versions after series of alt-tabs and other window switchings
• working with the wrong language, and thus updating .groovy files instead of .bsh
• not realizing I was working on a script that was located in another directory
  ...so I decided it was time to stop losing time while using the script editor in its current state and instead upgrade it somewhat.

So I implemented the following changes :

• I disabled the language switcher as soon as a script extension was known (either from load, save, or creating a script in a given language). The language still updates automatically when you use the "load..." and "save as..." buttons and change the file extension, but you can't manually set it. The only moment you can actually use this language switcher is when you use the "Edit script..." option for creating a new script, before you save it for the first time. I also added a submenu for creating new scripts and selecting the language from the menu. I thougt people usually know which language they will be using before they start programming :).
  
• I changed the "Tool / Edit script..." menu, and added sub-menus for creating new scripts, editing recently saved ones, and I moved the existing "Edit script..." one step farther in the menus hierarchy, because in practice, I found that most of the time I was going back and forth between sets of scripts using the "recent scripts" submenu, and it felt more natural to group all script-creating menu items (new, recent, and "edit script..") at the same level.
  
• when an editor window is opened on a script that is already opened in another editor, an alert box is displayed, and the script is opened read-only, with a grey background. This has already saved me from overwriting changes in my own scripts...
  I didn't disable the "save as... " button because you may want to create a copy of the script with a different name.
  
• I overhauled a little the editor window, so that, like in most development software, the load / save / execute buttons are located on top of the edited text. I also added a "save" button that just saves the file at its current location, without opening a file chooser, and is disabled until you type something in the editor window and so change its content. And for debugging purposes, I added the "execute to cursor" and "execute selected" buttons. I plan to add some more debugging facilities, like some autocomplete, links from the output stream into the code, breakpoints / watchpoints, keyboard shortcuts for some of the buttons.. The potential TODO list is long :/
  
• I have set the editor title to the full path of the script being edited (this has also helped me not getting confused about which directory the script I was editing was in..)
• When existing scripts are edited, the editor windows don't overlap exactly at the center of the screen : they are created a little below and on the right of all other windows original positions, making it impossible to mistake one editor for another.

All in all, there are a few quirks to work out (I want to add an alert if trying to close a script that has been modified but not saved, for example ; there is an issue when maximizing the editor window, some things need translations, etc.), but the changes I made have already helped me a lot when editing scripts.

I'd like to know if this, when polished, would make for an acceptable PR.
Maybe some or all of those changes could later make their way into the Scripted Object editors.
I'm joining a .jar (with an added .zip extension because GitHub doesn't allow jar files uploads, just remove the .zip in the file name - there is no need to unzip) in case people would want to try those changes out (and hopefully find bugs). I suppose simply replacing the ArtOfIllusion.jar original file with this one should be enough to use it..?

ArtOfIllusion.jar.zip

I would happily receive comments and critics about those changes. Open fire!

from artofillusion.

These actually look like they would be some handy changes.

I'd like to know if this, when polished, would make for an acceptable PR.

I'd say that the PR process should start before it's polished. The back-and-forth on the PR branch is part of the polishing process. Are you familiar with how to commit your changes to your GH repository and make a PR? If not, I'll be glad to help. (If you are really uncomfortable with the whole process, let me know. We can find another way to do things.)

If you are familiar with git and branches, I'd prefer to see these changes broken up into a couple different commits for ease of review:

• Your first 2 bullet points can probably go together, along with the last 2 -- I think.
• The read-only window should be separate. I would like to consider how this should be handled more broadly across the application.
• The editor controls overhaul (and extra buttons) should be a third. Some of the additions that you plan, such as the debugging stuff, should probably be a separate project.

If you haven't committed them separately, I'll be glad to help get them teased out. Let me know.

Maybe some or all of those changes could later make their way into the Scripted Object editors

Another good reason to start reviewing them in their rough state. This might mean re-working an entire approach at the code level, so best not to get too stuck in the details too early.

I want to add an alert if trying to close a script that has been modified but not saved,

Definitely. Confirm-on-close.

Couple of things I noticed on a quick spin with your jar:

• The "edit recent" as currently implemented really means "edit something that I've already opened this session" - it doesn't survive app close and restart.
• Having all of the buttons on top feels jarring compared to the rest of the application. AOI's UI design conventions are open to discussion, but we should try for consistency across the app. Keep in mind that most of AOI's audience are non-technical people who are not familiar with "Most development software".
• I'm not sure how I feel about the lack of an explicit "Cancel", as in "Get me out of here without trying to save" button. Yeah, it might be redundant. How will it feel for the primary audience, though?

Discussion for these questions would really be best suited to a PR thread for the code in question.

from artofillusion.

It appears I need a bit of clarification about the PR process and workflow : I have been brain-formatted to using CVS / SVN in an environment with a totally different organization, and I'm starting to realize this knowledge does not apply directly to GitHub development (I thought adapting would be a breeze :/).. So, here's what I think I understand now - correct me if I'm wrong?

For a given feature (let's say the save / saveas / load buttons, new script, recent script submenus, and language switcher behaviour), I :

• work on my fork of AOI, creating a branch with a name of my choosing for this given feature and committing changes from my local computer to my fork of AOI
• create a PR (in the AOI original repository) for this feature and link it to the given branch (I may be lost here)
• collect feedback, keep committing my changes and mergin with the changes pulled from master
• repeat until the PR is actually merged

As a side note, there are plenty of information about pull requests and branches everywhere, but ..TMI! A simple, shorter version like the one above would be very much welcome if it could be found in, say, an AOI Wiki page wink.

Also, when you say debugging features should be a separate project, do you mean PR? Class? Plugin? Something else?

from artofillusion.

Very close.

create a PR (in the AOI original repository) for this feature and link it to the given branch (I may be lost here)

This is actually initiated from the UI when viewing your branch. You'll see it when you have a branch to work from.

collect feedback, keep committing my changes and mergin with the changes pulled from master

Feedback and committing your changes, yes. Please do not merge in changes from master. If your changes start to conflict with changes in master, we will do something called a rebase. It's not hard, but don't worry about that unless it comes up.

A short overview of how to PR to AOI might well be worth the time.

when you say debugging features should be a separate project, do you mean PR? Class? Plugin? Something else?

In this case, separate PR. Possibly after the dust has settled on these changes, if they would overlap. (If you think that it would be possible as a plugin without major changes in core, great. I don't really expect that.)

from artofillusion.

Feedback and committing your changes, yes. Please do not merge in changes from master. If your changes start to conflict with changes in master, we will do something called a rebase. It's not hard, but don't worry about that unless it comes up.

Oh, that one is an important change to the workflow I was used to - we had to merge the code with the main branch, and only commit non-breaking changes so that the CI process would run nightly and test results were available next morning.
I'm not worried about rebases - I just downloaded 17 books about Git.

(If you think that it would be possible as a plugin without major changes in core, great. I don't really expect that.)

Honestly.. I hoped you would say PR :)
As a general comment, I think the Scripted objects and Tool Scripts should share a common Script Editor, and additional debugging tools could be added as needed : for the Tool Scripts, those could be watchpoints / breakpoints ; and for Scripted Objects, for example, sliders / knobs for the object parameters. I haven't been using Scripted Objects, so I am not sure what would be actually, really useful, but since they can take parameters, I guess turning a knob or moving a cursor would be more practical than clicking on a parameter, entering a value, clicking ok, repeat. I'll have a better opinion about this when I have created a bunch of Scripted Objects, but doesn't this sound like a good idea?

Ok, back to the comments about my changes :

• About the read-only mode : Turning this this feature into a "only-one-window-per-script" version as has been talked about earlier can be done by just changing half a dozen lines, but couldn't this be a user option instead? You see, when writing scripts (I'm talking about Tool Scripts here), I definitively want to be able to compare the result of an original script to an updated one by executing them side by side -meaning two opened windows- and still be sure which one is the updated one when saving. I don't think that's applicable to Scripted Objects though, so maybe the logic would be different here.
• I didn't realize AOI handles a list of recently opened scenes. I'll do a similar thing with scripts.
• About the "jarring buttons on top" : Actually, I thought having the editing buttons on the bottom of the text editing window was jarring :). After all, having editing and file management tools on top of the edited text area is standard in office software used by "non-technical" people (Who are they? Do they exist? :).. And if you consider consistency compared to the rest of AOI... the main window and mesh editor windows have icons on the left and editing functions/menus on top ; the textures and materials management window has all the buttons on the right (and would honestly require an ergonomy/usability overhaul) and the preview in the middle ; the texture editor itself has ok/cancel on top and preview on the right..
  So, when I did this change, I wasn't sure what kind of layout was ok but I definitively appreciated to grab a couple more lines of script text by moving the buttons on the same row as the language switcher, so in the end I copied the layout that can be found in pretty much any text editing software (hi jEdit users!). I mentioned development software only because developers are my intended readers :).
  Anyway, a simple way to make this window look more similar to other AoI windows would be to introduce a "Script" or "File" menu, with submenus "File/Load...", "File/Save", "File/Save as..", "File/Exit editor". How do you feel about this?
• About removing the "Close" button : I agree, I'm not sure about this change either ; but as a general rule, I don't like redundancy or ambiguity and I pictured people could think closing the window with the cross button and with the "Close" button were different, and that's ambiguous, so I removed the button. Should I add a "Cancel changes and quit" button? Make it into a menu as I mentioned earlier?

from artofillusion.

we had to merge the code with the main branch, and only commit non-breaking changes so that the CI process would run nightly and test results were available next morning.

With a test-suite that runs in seconds - currently, and even if it grows, it will never take overnight - I can do a local check before merging. Merging the master changes into the branch leads to a messier commit history.

Again, good ideas for the future. They deserve their own, separate discussions.

• R/O mode: you may have a point. If the change is that simple, we can certainly afford to drop it on a nightly, and get real-world feedback before heading to stable.
• Buttons-On-Top: You are right that AOI does have a variety of different layouts. I confess that I was mostly thinking of script editing in comparison to the various Object Editors. Thanks for the reminder. Other discussion on ergonomics/trying for a unified design should be its own topic. (Though I agree that T/M manager has a lot going on, and might benefit from a more streamlined design. Separate issue/brainstorming thread!)
  • Not so sure that we'd want to go for more menus, that is one thing that I'm not crazy about in the current design. How to minimize them is the question...
• We can try it as-is in nightly, I think. Don't think we need a long-name even if we do add the button back. (That's what "Cancel" means)

from artofillusion.