search

tim-janik / beast

Beast - Music Synthesizer and Composer
GNU Lesser General Public License v2.1
84 stars 12 forks source link

Bus properties ported to C++ - redone with APPLY_IDL_PROPERTY() macro #105

Closed swesterfeld closed 5 years ago

swesterfeld commented 5 years ago

As requested, I went through all changes in https://github.com/tim-janik/beast/pull/78 and used APPLY_IDL_PROPERTY() where possible. In some cases it wasn't possible, so there are a few handwritten setters. I also avoid any fancy new features in the code or ui, this is an 1:1 port.

This also fixes the C++ property loader code in bsestorage.cc so that make check passes.

From my perspective the only thing that could be controversial is that you used g_object_set(...property...value...) to avoid undo recording for some setter calls. I added a function to block undo manually so

  g_object_set (self, /* no undo */
               "sync", self->saved_sync,
                NULL);

now looks somewhat like this

  bus->block_property_undo(); 
  bus->sync (self->saved_sync);
  bus->unblock_property_undo();

This should work in the same way for setters that use APPLY_IDL_PROPERTY() and for hand-written setters that use push_property_undo(). If you have a better suggestion feel free to either change this after merging or letting me know how it should be done.

swesterfeld commented 5 years ago

Please fix review comments I left for you and remove: a) the commit introducing the block/unblock API,

Done with all code that depends on it.

b) the float64 workaround.

Done.

If that means you updated PR is broken, point out the bugs you're runing into and I'll give you a hand with the fixes. In particualy, I'm not even sure you can trigger buggy behaviour due to not blocking undo, if you find a way to trigger that, that'd be really helpful.

I haven't ever seen any undo related problem while testing it.

As for fixing the float parsing, I looked into that earlier and am quite confident I can fix parse_paren_rest. I just needed an actual test case that triggered breakage, which I didn't have at the time.

Right. make check is broken now due to audio tests, so before merge, you should look into fixing the parser.