Skip to content
This repository has been archived by the owner on Apr 18, 2022. It is now read-only.

Module organization - Seperation of concerns #191

Closed
lschmierer opened this issue Feb 15, 2017 · 6 comments
Closed

Module organization - Seperation of concerns #191

lschmierer opened this issue Feb 15, 2017 · 6 comments
Labels
diff: normal Achievable by an reasonable experienced developer. If new to Amethyst, may need some guidance. pri: important Something other teams are relying on, or a low-level, critical piece of functionality. type: improvement An improvement or change to an existing feature.
Projects

Comments

@lschmierer
Copy link
Contributor

I would advocate for organizing the modules of the amethyst crate by "features".

So instead of the current structure like,

* ecs
  * components
    * rendering
    * transform
    * ...
  * systems
    * rendering
    * transform
    * ...
  * ressources
    * ...

I would suggest something like

* rendering
  * ecs
    * components
    * systems
    * ressources
* transform
  * ecs
    * components
    * systems
    * ressources
* ...

where everything that directly belongs together can be found in one place.

@nchashch
Copy link
Member

If we adopted this approach it would make sense to drop ecs submodule, so it would look like:

* rendering
  * components
  * systems
  * resources
* transform
  * components
  * systems
  * resources
* ...

@ebkalderon ebkalderon added diff: normal Achievable by an reasonable experienced developer. If new to Amethyst, may need some guidance. pri: important Something other teams are relying on, or a low-level, critical piece of functionality. type: improvement An improvement or change to an existing feature. labels Feb 15, 2017
@lschmierer
Copy link
Contributor Author

To avoid confusion, the amethyst_renderer crate should probably moved out of the amethyst crates src directory.

See #121.

@ebkalderon ebkalderon added this to New in Engine Feb 15, 2017
@ebkalderon
Copy link
Member

Hmm, I am inclined to disagree. I see the following issues with this approach:

  1. Regarding components and resources that are shared between systems: the Transform, Rendering, and Physics systems might all be interested in the Transform component. Which subfolder should it go under?
  2. The Transform system is an essential part of the engine and is not a "feature" in the Cargo sense, if that is what you were implying. If I misinterpreted your statement, please ignore this complaint.
  3. What about @Aceeri's upcoming project module? It sits on top of everything, whether ECS-related or not. Another example is the engine module. Is it really a good idea to mix these modules in with all the ECS stuff?
  4. It goes against idiomatic Rust style. The norm is to have data structures be placed directly alongside the functions and methods that operate on them. I think that having dedicated modules only for data (components), mixed data and logic (resources), and almost only logic (systems) flies in the face of this idea. Besides, traversing up and down a deeply nested hierarchy which might even mislead people (see issue 1 above) looking for what they want is very annoying.

@lschmierer
Copy link
Contributor Author

  1. Regarding components and resources that are shared between systems: the Transform, Rendering, and Physics systems might all be interested in the Transform component. Which subfolder should it go under?

Probably under transform together with the Transform system. Rendering shard then depends on Transform shard.

  1. The Transform system is an essential part of the engine and is not a "feature" in the Cargo sense, if that is what you were implying. If I misinterpreted your statement, please ignore this complaint.

One could interpret/implement Transform as a (feature-gateable) shard if one likes to, however I tend to agree with you that it should better be core funcionality.
But it still kind of provides cpupled functinality (Transform system and Transform components).
Therefore I would still put in in a seperate transform module.

  1. What about @Aceeri's upcoming project module? It sits on top of everything, whether ECS-related or not. Another example is the engine module. Is it really a good idea to mix these modules in with all the ECS stuff?

To distinguish ECS stuff, I would still, different to what @nchashch suggested, keep an ecs module inside of the feature modules, like:

* transform
  * ecs
    * components
    * systems
    * ressources
  * other non-ecs stuff

instead of:

* transform
  * components
  * systems
  * ressources
  * other non-ecs stuff
  1. It goes against idiomatic Rust style. The norm is to have data structures be placed directly alongside the functions and methods that operate on them.

Maybe I don't get the point, but I think in my proposal data structures and corresponding functions are much closer than in th current approach.
Currently we might end up with one big folder of (maybe hundreds of) components and a big folder of dozens of systems.
My approach is not more deeply nested as it curently is.
It is just another approach by grouping by features in the first place instead of by type.

After all, this probably a matter of personal preference.

@ebkalderon ebkalderon mentioned this issue May 7, 2017
12 tasks
ebkalderon added a commit to ebkalderon/amethyst that referenced this issue May 8, 2017
ebkalderon added a commit to ebkalderon/amethyst that referenced this issue Jul 1, 2017
@lschmierer
Copy link
Contributor Author

@ebkalderon I think this can be closed?

To summarize: We keep ecs components and systems in the ecs module like it is now.
After thiking about it again, my proposal of modularization by feature would not give big advantage (if at all somehow feasible) since e.g. the most components will be used by multiple systems and it would not be clear where it would originally belong to.

@torkleyy
Copy link
Member

Agree with @lschmierer, closing.

@ebkalderon ebkalderon moved this from New to Done in Engine Jul 21, 2017
ebkalderon added a commit to ebkalderon/amethyst that referenced this issue Aug 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
diff: normal Achievable by an reasonable experienced developer. If new to Amethyst, may need some guidance. pri: important Something other teams are relying on, or a low-level, critical piece of functionality. type: improvement An improvement or change to an existing feature.
Projects
No open projects
Engine
  
Done
Development

No branches or pull requests

4 participants