From our last BseSong merge:
There's one other thing I noticed during review. With the old GParamSpec + GObject based property API, the GObject machinery ensures that the GValue passed in to property setters complies with the GParamSpec value range. With our IDL API, nothing like that is enforced anymore. That means, an int32 property between 1-256 can now be set to 0 or -2^31 through the API. For a denominator to become 0 that could affect stability.
Please keep that in mind for ported properties and watch out if we shouldn't add extra guards, like:denominator = CLAMP (input, 0, 256)
I think it might be better to re-add range enforcement. Right now, I would have to write this setter code:
void
SongImpl::denominator (int val)
{
BseSong *self = as<BseSong*>();
val = CLAMP (val, 1, 256);
if (int (self->denominator) != val)
{
const char *prop = "denominator";
push_property_undo (prop);
self->denominator = val <= 2 ? val : 1 << g_bit_storage (val - 1);
bse_song_update_tpsi_SL (self);
notify (prop);
}
}
Note that we duplicate the defaults here, which is not ideal. So we could also check against the pspec, somewhat like
...
val = prop_denominator.clamp (val);
// prop_denominator would be generated and contains min(), max(), def(), name()
...
But I think ultimately what I really want to write is this code:
void
SongImpl::denominator_impl (int val)
{
BseSong *self = as<BseSong*>();
self->denominator = val <= 2 ? val : 1 << g_bit_storage (val - 1);
bse_song_update_tpsi_SL (self);
}
and have the actual denominator (int val) function be generated automatically, which should
Consider this a brianstorming idea I at least wanted to share. Maybe the actual implementation will be a bit different, but I think the general idea to have each setter always execute some steps at the beginning and some steps at the end of setting a property is likely to make our actual code simpler.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.