search

EcsRx / ecsrx.unity

A simple framework for unity using the ECS paradigm but with unirx for fully reactive systems.
MIT License
417 stars 65 forks source link

How about refactoring out all the 'new' statements which make it hard to replace functionalities through plugins? #23

Closed gregee123 closed 7 years ago

gregee123 commented 7 years ago

Pool has a hardcoded 'new Entity()', PoolManager has a hardcoded 'new Pool()'. If instead those classes used the simplest injectable zenject factories it would be easier to provide different implementations of IEntity, IPool without having to replace the whole chain PoolManager->Pool->Entity.

Heck, maybe CashableGroupAccessor or even GroupAccessors implementations in general should be easliy pluggable?

Zenject factories are practically one line classes (really more like typedefs in c++ speak) so should be easy enough.

grofit commented 7 years ago

Sure, will look into this for next release, I just didnt want to litter lots of factory classes all over the place, and in most cases if you were going to change one you would change the lot so the interfaces were the point of contract for those things, so you can easily replace high level classes with your own implementation.

Anyway let me look over and report back as like you say Zenject does most of the stuff for you in this area.

grofit commented 7 years ago

After looking through the zenject docs more around factories I very much dislike their default approach of using an inner class on a concrete type, as that doesnt seem to give you any more flexibility as you are still tied to the concrete class and its awkward to maintain inner classes.

The more general Factory<T> type stuff is a bit nicer but realistically you will end up not being able to mock out those components as easy as they have a base inheritance path, so I think I will just introduce an IFactory<T> and have implementations for that so if you want to use your own factory you can just override that and put it into the DI config.

== Edit ==

Didnt notice there is an IFactory within there, but one major thing I have just realised is that currently Zenject is only required on the Unity project not on the Core, and I wouldnt want to introduce Zenject into the core, so for now I will just have an IFactory within the core project as its hardly any code to do, and it keeps the core unknowing when it comes to how it is all composed.

gregee123 commented 7 years ago

Zenject was just a suggestion. You can introduce your own factory types / contracts. My main concern was hard-coding concrete types within methods.

grofit commented 7 years ago

yeah valid concern, as mentioned I originally envisioned people would just implement their own stuff as Entity and Pool implementations are quite small BUT the pool manager dependency is a bit bigger, anyway its pretty much sorted locally so will push that up later once I have tested it some more.

grofit commented 7 years ago

This has been released in 0.8.0 feel free to re-open if there are any issues with this