The refcounting scheme used by Action was not working very well; Actions would essentially Dispose themselves as soon as they were no longer referenced by any ActionLists. This makes Action management very tricky for the API consumer: removing an Action from an ActionList and then subsequently adding it to a different ActionList would trigger a disposal.
OTOH, the Action architecture as-is also needs to map action IDs to Action pointers. As package-scoped maps, these entries hinder GC and would require every single Action to be explicitly Disposed. For complex menus, this would be pretty ugly to manage.
In this PR we keep the refcounting, but we only use it for the package-scoped maps: when the refcount becomes positive, we add the action to those maps. When the refcount reaches zero, we remove the action from those maps. While we do offer an explicit Dispose method, we also add a finalizer that will handle the complex menu case.
Furthermore, Dispose does not release the Action ID; only the finalizer does this. This ensures that there is no possibility of ID reuse as long as the application holds references to an Action with that ID.
I also made a fix to menu.go: submenu items may also be owner-drawn or have images, so recursing into updateItemsForWindow should not be mutually-exclusive to the former two conditions.
…n lifecycle and change management
The refcounting scheme used by Action was not working very well; Actions would essentially Dispose themselves as soon as they were no longer referenced by any ActionLists. This makes Action management very tricky for the API consumer: removing an Action from an ActionList and then subsequently adding it to a different ActionList would trigger a disposal.
OTOH, the Action architecture as-is also needs to map action IDs to Action pointers. As package-scoped maps, these entries hinder GC and would require every single Action to be explicitly Disposed. For complex menus, this would be pretty ugly to manage.
In this PR we keep the refcounting, but we only use it for the package-scoped maps: when the refcount becomes positive, we add the action to those maps. When the refcount reaches zero, we remove the action from those maps. While we do offer an explicit Dispose method, we also add a finalizer that will handle the complex menu case.
Furthermore, Dispose does not release the Action ID; only the finalizer does this. This ensures that there is no possibility of ID reuse as long as the application holds references to an Action with that ID.
I also made a fix to menu.go: submenu items may also be owner-drawn or have images, so recursing into updateItemsForWindow should not be mutually-exclusive to the former two conditions.
Fixes #55