Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

final design decisions before v1.0 #43

Closed
13 of 14 tasks
Ivshti opened this issue Jun 1, 2019 · 1 comment
Closed
13 of 14 tasks

final design decisions before v1.0 #43

Ivshti opened this issue Jun 1, 2019 · 1 comment

Comments

@Ivshti
Copy link
Member

Ivshti commented Jun 1, 2019

in light of using this in stremio-web, stremio-example-seed, stremio-example-sauron, stremio-example-ui and stremio-ng-example,

there are a few more design decisions to make before finalizing:

  • instead of CatalogMuxer, implement a #[derive] macro that will allow the user to fill a struct with fields that implement Container; that is much more ergonomic; however, this struct should not have interior mutability, and mutability will need to be handled by the user (implementation-specific) - DOING IT
  • should internal APIs use future Streams instead of callbacks - DOING IT
  • redux middleware style vs elm-style update function - GOING FOR ELM STYLE
    • elm-style update problem: many things are dependent on the user ctx, so we will have to somehow retrieve it before determining what tasks to issue; if we do a promise of the style with_ctx, then it's kinda ugly (adding async logic to update)
      • or, simple solution: we do update(&mut Model, &Ctx) -> Effects
      • we can still have loopback actions
    • a good reason for loopback actions is that you might want some environment side-effects, e.g. routing; receiving a RecommendedXXX (as a result to Open) to load should lead to a page that will trigger this Load
  • consider a AddonEffects trait (with a fn that returns AggrRequest) instead of addon_aggr_req; and in general, a mechanisms for Containers to define thier own Load parameters and effects; could be a trait associated type that we use in Actions - not needed
  • ...or, a more generalized Effects trait with fn effects(ctx: Ctx) -> Vec<EnvFuture<Action>> - not needed
  • should the update function mutate the struct or return a new struct? - mutate
  • consider StandardStremio - a struct with all containers a usual stremio app has - not needed, derive macro handles this great
  • AddonM can be enhanced with memoization to avoid re-requesting the same thing (e.g. when going from /detail/movie/tt213 to /detail/movie/tt213/tt213), but for now there's no point of doing this cause (1) the reducer will keep the old response and (2) the browser has a cache - not a problem with elm architecture
  • Problem: circular dependency: player needs addon system responses (from AddonM) but needs to emit to the library add-on (back in ContextM) or control it somehow; LibAddon itself needs to be connected after AddonM (to listen to AddonRequest/AddonResponse) - not a problem with elm architecture
    • solution 1: re-think the chain
    • solution 2: drop the chain, there'd be one middleware that manages everything
    • solution 3: use a loopback action
    • solution 4: the player should somehow capture a handle ot the LibAddon and use it directly
    • solution 5: the elm architecture (Player update will handle it's own side effects)
  • do we need a common trait between MetaItem, MetaPreview and LibItem? - a common trait between LibItem/MetaPreview/MetaItem #53
  • do we need LibItemPreview? - no
  • when saving the last stream, save the whole stream object but compressed OR save the stream hash; hash can be saved within the libitem - whole stream makes more sense, could be part of PlayerPreferences?; this decision may change
  • design decision: models (containers; or should we perhaps rename them to models) should contain raw, but complete data; their update function should focus on how they mutate, and do as little processing as possible; they will do sorting and filtering (e.g. Search) though
  • figure out environment-specific settings; perhaps use the Environment trait?

see also: #28

@Ivshti Ivshti pinned this issue Jun 1, 2019
@Ivshti
Copy link
Member Author

Ivshti commented Jun 5, 2019

Due to many complicated bits, like the player and detail page, we're most likely elm-ing it.

  • we can do update(&mut self, &Ctx) for regular containers
  • the Ctx itself will be a container that exposes an update(&mut self, &action)
  • the Ctx container may actually become cleaner than the contextM since we'll just make a message Internal::UpdateCtx and eliminate the need for interior mutability
  • a #[derive] macro will allow us to compose many types that implement update
  • this will compose in nicely with elm-like frameworks (seed-rs) but it will also compose great with React/Conrod/whatever with a bit of glue: create a struct that manages mutability and Ctx; we can also provide glue:
    • something that will return a channel Sender (for actions) and a Receiver (for new states and other outputs, e.g. errors), and will manage context and will consume a struct that implements update
  • LibAddon's interior mutability might need to be re-thought too
  • we should probably sync the library on log-in; It makes a lot of sense for UX

Example API:

// TODO all those will have to take a type arg for Env....
#[derive(StremioModel)]
pub struct StremioModel {
   pub discover: CatalogFiltered,
   pub board: CatalogGrouped,
   pub player: Player<PlayerImpl>
}

let mut stremio = StremioModel::new();
let ctx = Ctx::new();
stremio.update(&ctx); // will return a Vec of futures that have to be handled

// or more sophisticated
// TODO: figure out a better name for this
let (tx, rx) = App::with_managed_context::<Env>();
// send actions to tx
//rx.for_each() // do something with the new &model

NOTE: the models might have to take AddonTransport besides Env

@Ivshti Ivshti closed this as completed Jun 6, 2019
@Ivshti Ivshti unpinned this issue Jun 6, 2019
@Ivshti Ivshti mentioned this issue Jun 12, 2019
elpiel pushed a commit that referenced this issue Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant