closebang bug fixes

[Spacechild1]

• send closebangs before any object is actually removed and in the same order as loadbang and initbang:

1. abstractions / clone objects (recursively)
2. subpatches (recursively)
3. remaining objects on the canvas

the current ordering is messy because when an object receives a closebang, other objects might have already been destroyed. so if you want to save a table with [closebang] for example, you have to put it into the right place in the glist (the details are complicated). here's a patch which demonstrates the problem:
closebang-issue.zip
what I would want instead is that no object is actually deleted unless all closebang messages have been sent, so I don't have worry about where I put my [closebang] objects.

• don't send closebang twice to cloned abstractions

• fix a bug where non-canvas objects on the root canvas wouldn't receive a closebang message.

---

externals which use glist_delete from the private canvas API still work as expected.
the [clear( message also works.
cutting objects is ok but canvas_closebang is called on every selected canvas in series - which means the closebang order is only correct for each selected canvas. otoh, it's the same with loadbang and initbang when pasting multiple canvases, so at least its consistent. later we might want to fix the initialization order for cut&paste...

here's a test patch (needs [iemguts] with a version before 2018):
closebang-test.zip

needs review from @umlaeute (see also https://git.iem.at/pd/iemguts/issues/8)

[Spacechild1]

@umlaeute gentle reminder ;-)

[Spacechild1]

@umlaeute if you have time, it would be great if you could have a look, since this PR is mainly aimed at [iemguts/closebang].

[umlaeute]

fine by me.

[millerpuckette]

Would it work to change canvas_loadbang(t_canvas *x) to canvas_loadbang(t_canvas *x, t_floatarg action) where
acvtion can be LB_LOAD or LB_CLOSE? Then canvas_free could just blithely call canvas_loadbang(x, LB_CLOSE) and we could tell anyone wanting to catch the CLOSE message that it can happen multiple times but just ignore all but the first one you get.

[Spacechild1]

We can't really send the "closebang" from within canvas_free because it would mess up the ordering, as explained in the PR description. Most importantly, I make sure that all "closebangs" are sent before any objects are actually deleted, just like "loadbangs" are sent after all objects have been created.

[millerpuckette]

I think this should be done differently by writing a unifying function that can recurse into all patces/subpatches with options to go depth-first and/or cancel going into abstractions/clones and/or only report the first one after a given match and perhaps others as needed to support find, loadbang, closebang, and all that. I'm afraid that putting features in piecemeal for 0.52 will make that harder to do compatibly in the future, so I propose to try to get this running after 0.52 goes out (hopefully ready to 'test' very soon).

[Spacechild1]

Ok! I'll keep this PR open just as a reference for the "correct" closebang behavior. Once you've implemented your solution, it can be closed.

[Spacechild1]

I'm afraid that putting features in piecemeal for 0.52 will make that harder to do compatibly in the future, so I propose to try to get this running after 0.52 goes out (hopefully ready to 'test' very soon).

On a second thought, unified canvas traversal is just an implementation detail, the actual feature (= behavior) would be the same. So I don't really understand what you mean by "putting features in piecemeal".

Specifically, if you merge this PR, you would later only need to change the implementation of canvas_closebang, just like canvas_loadbang and canvas_initbang. The function itself, however, would already be called at the right places - which is the actual tricky part.

I think it could be helpful to have the right behavior in place before doing all the refactoring. But that's just my opinion. Do whatever you feel more comfortable with :-)