client-toolkit icon indicating copy to clipboard operation
client-toolkit copied to clipboard

Miscellaneous inconsistencies

Open ids1024 opened this issue 2 years ago • 0 comments

There are a few inconsistencies in sctk currently. That might be worth thinking about for the next breaking version.

  • It would make sense if ProvidesBoundGlobal were implemented for every type in the crate that provides a global.
    • But by the requirements of the trait, as defined, it can't be implemented for things like dmabuf, for which sctk supports versions 3 and 4. Both of which seem important to support now.
      • Could be defined as an enum of states by version, each of which implements ProvidesBoundGlobal for different versions, or methods returning things that return ProvidesBoundGlobal, but those are awkward...
  • Some wrappers around globals have names ending in *State but some don't.
  • Some have a constructor called ::new while others have one called ::bind
  • Handling of protocols/globals that may or may not exist
    • Shm::bind returns an error if the global doesn't exist; but implementing ShmHandler requires returning the Shm (I guess if the protocol somehow is optional for a client, it could store an Option<Shm>, and define shm_state with .as_mut().unwrap()?)
    • Some other state types have constructors that always succeed, but methods returning a GlobalError result. ProvidesBoundGlobal handles this. Methods may fail regardless if they need a newer protocol version.

The use of methods on state types as helpers for sending requests is occasionally inconvenient if one wants to send requests from another thread... though in the worst cast those can be done directly. Not sure if there's a good way to define helpers than handle this case well.

ids1024 avatar Oct 19 '23 01:10 ids1024