Open gedankenexperimenter opened 6 years ago
Note that it's probably not necessary for all plugins to implement this system for it to work (though I do think it's a good idea).
As the ID does not need to be persistent, wouldn't it be possible to use the plugin class address as an ID? Granted, that's 16 bits wide, but it is something we already have access to.
Interesting idea; I hadn't thought of that. I'm not entirely convinced that it's better than my original idea, though, which is only one byte per (registered) plugin, plus one byte to compare (twice) every time a hook function (that checks) is called, vs two bytes to compare (twice) for every hook function call.
I suppose some plugin hook functions would only need to check once (if they should process events only once, but never send injected events), and others wouldn't need to bother to check at all (if it doesn't matter if they process events twice).
One other note; plugins that don't queue and release events, but generate artificial events that should be processed by other plugins (I'm not even sure if there are any such plugins, but there might be) will still need a way to avoid processing their own injected events, but allow the other plugins to do so. This scheme won't help with that, but it will still be possible for a plugin to use an internal variable to suppress event handling temporarily, and call the Kaleidoscope function for handling an event as if it was a normal key event.
I'm not sure if this would make the INJECTED
flag obsolete, but it could theoretically still be useful to signal to other plugins that an event was artificial, and not tied to a physical keypress, even if it's not used to short-circuit infinite loops.
As the ID does not need to be persistent, wouldn't it be possible to use the plugin class address as an ID? Granted, that's 16 bits wide, but it is something we already have access to.
I started out thinking of registering plugins, and having Kaleidoscope store an array of plugin object pointers instead of separate arrays of hook function pointers. I eventually dropped that idea, though it still might be useful if I ever try to build a sketch code generator to replace the awful incomprehensible TMP stuff with something more accessible to non-C++ fanatics.
At the moment, it looks like we'll end up with an array of plugin objects instead of separate hooks function arrays. Still working on keeping the template stuff within reasonable limits, but I'm fairly sure we can get there. Where reasonable limit is something like Kaleidoscope.use()
, and it's going to be something most people won't have to care about at all. It won't be visible outside of the Kaleidoscope class. (At least, that is the plan. Keep an eye on keyboardio/Kaleidoscope#276 in the next few days.)
Does this mean keyboardio/Kaleidoscope#276 is changing from the unrolled-loop-template-magic? I've been watching that, and putting other work of my own on hold for the moment while I wait for it to play out.
If there is an array of plugin object pointers, the index of the plugin's entry in that array could serve as the id #.
I also toyed with the idea of having different sets of numbers for each hook, allowing Kaleidoscope to simply start farther along through a loop, but that seemed like a lot of bookkeeping for dubious value.
I'm still considering using an array of plugin object pointers, but I haven't implemented it yet in Kaleidoglyph. I have implemented a version of @algernon's idea to compare pointers, but I used plugin object pointers (if I understand correctly, that idea was to use the address of the class, not the instance, but I've searched just now and can't find evidence that there's a way to do that).
Anyway, what I did was define a checkCaller()
function in the base Plugin
class, and added a Plugin*& caller
parameter to the keyswitchEventHook()
function. checkCaller()
(I'd prefer a better name) looks at the caller
pointer: if it's nullptr
, it does nothing, and lets the hook function proceed; if it's this
, it sets caller
to nullptr
and stops the hook function; otherwise, it aborts the hook function. So if a plugin wants to re-send an event that has already been processed by itself (and all the plugins before it), it calls handleKeyswitchEvent(event, this)
. If it needs to send a new injected event (that should be processed by all plugins), it calls handleKeyswitchEvent(event, nullptr)
, and if it needs to ignore that event, some other mechanism must be employed.
If there are multiple plugins that behave like Qukeys – intercepting events, queueing them, then releasing them later for further processing – we need a better mechanism than the
INJECTED
flag for avoiding infinite loops and making sure each event gets processed by all the plugins it should.A single bit won't suffice, because each plugin should ignore its own injected events, but not those injected by other plugins. If two queueing plugins operate on the same hook point, the first one would intercept the event, call the handler again to continue and mark it as injected. The second one would then ignore that event, effectively never seeing it.
If we don't mark the event injected, then when the second queueing plugin releases an event, it goes back through the hook loop again, and the first one will see it, and queue that event, causing an infinite loop. This will happen even if we use some internal strategy in a plugin to ignore its own injected events.
My solution to this problem is to have the plugins all register with Kaleidoscope on init, acquiring a unique ID number. Then, when a plugin calls the event handler again to release a queued event, it can pass its ID number as a parameter to the event handler. The hook function would also take that number as a parameter – by reference – and if the ID number isn't the special "no plugin" value, the plugin shouldn't act on that event, because it has already processed it. If the ID number matches the plugin whose hook function is being called, it will overwrite that ID number with "no plugin" before returning. If all plugins do this, each plugin only processes an event one time. I'd name this additional parameter
caller_id
, with Kaleidoscope itself getting the special value of zero.