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

System::setup() ergonomics #437

Closed
kazimuth opened this issue Jun 2, 2018 · 5 comments
Closed

System::setup() ergonomics #437

kazimuth opened this issue Jun 2, 2018 · 5 comments

Comments

@kazimuth
Copy link

kazimuth commented Jun 2, 2018

System::setup() is currently slightly awkward to use, for a few reasons:

  • It takes a shred::Resources instead of a specs::World, which has a different and unfamiliar API from everything else in your code (because System is just a reexport of shred::System). This has consistently tripped me up; it would be nice if it was possible to just use World (or Resources) everywhere.

  • It's operating on an already-initialized version of your data, which means that if you want to load stuff in setup you have to put unwrap()s everywhere in run() to get access to fields that you know are present. One way to fix this would be to let run() return an error, which would let you use self.prop? instead of self.prop.unwrap() everywhere. Alternatively, it might be possible to make something where you can add struct initialization options to a Dispatcher and trust it to initialize the struct later.

These are minor nitpicks, not a big deal, but setup() is used a lot so it's good to make it ergonomic :)

@torkleyy
Copy link
Member

torkleyy commented Aug 5, 2018

I agree that it could be more ergonomic. There was no intention to make setup used that often actually, so I didn't think it was a problem. The first issue seems rather unsolvable to me (except you rewrite a lot of the structure of these libraries) and the second one I'm also not so sure about. I think implementing that would require the world for building the dispatcher, plus we'd need to pass custom args to the system in a tuple (so the user can initialize fields of the system, too).

@Aceeri
Copy link
Member

Aceeri commented Aug 21, 2018

Maybe something like this for an alternative setup?
http://play.rust-lang.org/?gist=4c20451d86601bf650ceb973762fc035&version=nightly&mode=debug&edition=2015

Would be a bit boilerplatey and requires constant associated types for less breakage.

@torkleyy
Copy link
Member

@Aceeri That link seems to be broken for me.

@torkleyy
Copy link
Member

torkleyy commented Jan 3, 2019

I think this issue consists of two parts:

  1. World / Resources mix; addressed by Make World an extension of Resources #523
  2. Non-trivial initialization; I think setup should not be used for that. Initialization has such diverse requirements, any attempt to wrap it in shred or Specs will most likely fail and / or make things overly complex.

Therefore I'd like to document that. Additionally, I'd like to consider deprecating custom setup / initializing system fields there.

@torkleyy
Copy link
Member

torkleyy commented Jan 3, 2019

Moved 2) to #525.

@torkleyy torkleyy closed this as completed Jan 3, 2019
@torkleyy torkleyy mentioned this issue Jan 15, 2019
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants