Design idea regarding Event::Command

[smmalis37]

The fact that it's possible for a user-defined command to accidentally collide with one of Druid's predefined commands doesn't feel great to me (as incredibly unlikely as it is). Therefore my first suggestion is for the Command variant to be split into two new Event varaints: DruidCommand and UserCommand (names subject to bikeshedding). DruidCommand can then become an enum with all the currently constant Selectors becoming variants instead.

My next thought was that it would feel more ergonomic to me if this new UserCommand type could become a user-defined enum, instead of using Any. It would likely require some additional threading of a new generic parameter, so this may not be worth it. But if it was done it could eliminate the Selector type entirely. We've got a type system, we should use it instead of just stringly typing things imo.

Both of these would be rather large breaking changes, so they may not be worth it. Thoughts?

[luleyleo]

Personally, I don't think it is worth the additional effort. Having a convention to just prefix all selectors with the crate name is just much simpler and does the trick. For example all commands offered by druid are called something like druid-builtin.some-command which would only collide if you really are looking for trouble.

Of course, if Rust should ever offer some ergonomic way to get access to constant, unique ids, we will switch to that for safety and ergonomics. Until then, we will probably have to live with string identifiers.

Thanks for writing down your ideas though!

[ratmice]

Hmm, I would have thought something like the following might have worked,
Alas, it fails the last assertion for constant values...

https://play.rust-lang.org/?version=beta&mode=release&edition=2018&gist=7d14d3d7eca390973af6127d01e290f2

Edit: It does actually work if we declare them static instead of const...

[luleyleo]

I've actually tried this approach during my search for an alternative for string id's and could not stop the compiler to optimize all selectors to a single pointer :/

MaybeUninit seems to do the trick, but is there any guarantee that this will never change? Or is it more of a side effect, that rustc can't optimize it right now?

Another issue is that both
static UHH: UniqueId = UniqueId::new();
and
const UHH: UniqueId = UniqueId::new();
compile but the latter will not behave as expected.

[ratmice]

MaybeUninit doesn't guarantee that the underling pointer will never change,
if you move the value it will absolutely change the underlying pointer along with the moved value.

Also i'm not particularly aware of the semantics defining std::ptr::eq, it does seem to be peculiar with both const, A third issue is that if you change it to MaybeUninit<()> unstead of bool I believe it'll also compare equal (what the inaccurate sized comment is about Zero sized is still sized).

Perhaps we can do something with std::pin::Pin to keep it from moving. But yes, the specific details of this pointer equality approach seems non-obvious regarding the results and guarantees.

I don't think its reasonable to expect though that rustc could ever optimize it away, since it pretty well hides the underlying allocation from the compiler. We could seek clarification I suppose if we decide this is an option worth pursuing.

[smmalis37]

When I initially came up with this idea I was thinking about the perf implications as well as the ergonomics, but you've mentioned that Selectors are actually just doing a pointer comparison and not a full string comparison, so that helps. I'm not sure what the cost of the any downcast is compared to an enum destructure, but probably not much.

However this raises an interesting issue to me. This style of pointer comparison only works if everything is statically compiled together. Now while that is the default in rust, compiling to a dylib is supported. If one were to define the same string in two different places, compile them to separate binaries/libraries, and have one load the other dynamically, these Selectors wouldn't match, despite the string contents being identical. This is an extremely unusual use case though, so I don't think it really matters. And to solve it you could just expose one of the strings publicly instead of defining it twice.

For an additional perspective on the enum based approach, take a look at the library Iced. The way they implement it is different from how it would have to be in Druid however. They have an Application trait that the root of your app implements, which has an associated type Message. That Message type then gets applied to all of your widgets inside the Application. Druid has no root-level construct like this.

[luleyleo]

@smmalis37

I was thinking about the perf implications

Honestly, if we manage to reach a point where comparison of command selectors becomes relevant, we will have already succeeded in building the fastest UI library in existence :)

Ergonomics is really the only reason I would be convinced to change this.

but you've mentioned that Selectors are actually just doing a pointer comparison and not a full string comparison

That's not quite right, Druid does a string comparison (which gets optimized with pointer comparison by Rust).
What @ratmice illustrated was what we could do if we went with pointers as id's.

That Message type then gets applied to all of your widgets inside the Application. Druid has no root-level construct like this.

Yeah, I don't think this approach would work for Druid. It could probably be done with something like parameterised modules but I doubt that will ever become a feature of Rust.

---

@ratmice

But yes, the specific details of this pointer equality approach seems non-obvious regarding the results and guarantees.

That's my biggest issue with this approach.

Another problem is debugging. Right now it is quite nice because every command has a unique, human-readable id that can easily be greped for. Using pointers you would probably still want to give your commands unique id's simply to be able to differentiate them when something goes wrong.

[madbonkey]

Druid has no root-level construct like this.

@smmalis37 Noob suggestion, but the delegate you register with the launcher strikes me as a pretty straightforward place to attach a potential Message type to. Or even making AppLauncher a trait for users to implement, and have it all on there? I don't know how all the *Ctx are implementen/constructed regarding commands, but given that there is some amount of plugging and plumbing at the top level anyway, why not encode it more coherently and also enable deeper control over command/message objects?

[cmyr]

I'd be curious to see a sketch of this, but my current feeling is that adding a top-level user-defined type is going to end up infecting most of the other type signatures in a pretty big way, and it isn't totally clear to me what the benefits are.

One possible exception to this would be to have there be a separate 'Event::AppMessage' event, that would be a single place where you can inject a type, and this would hopefully only change the signatures of the event fn and the Event enum? For now though I'm happy to be hacking around with Command, even if it doesn't feel 100% pure and good.

[PoignardAzur]

I'm going to close this. For now all druid commands have a name starting with druid-builtins.* or druid-tests.*, so I think the collision risks are negligible; and as other already pointed out, it's not really worth the cost of a refactor.

[maan2003]

also there is selectors macro in nursery. a selector generated by the macro will not collide with other selectors generated by the macro