hmans/miniplex

Dont throw an error when trying to remove component if it doesnt exist?

mikecann opened this issue · 4 comments

I usually am in favour of being more explicit with throwing errors when something is wrong but in this instance I wonder if it would be better just to carry on if the component doesnt exist rather than throwing an error?

https://github.com/hmans/miniplex/blob/main/packages/miniplex/src/World.ts#L207

Just makes things a little bit nicer to work with in user land (dont need to do a check before removing) and I cant see any instance where I really care that im trying to remove a non-existent component.

Please do let me know if im missing something tho.

In the case of adding components however im not sure..

https://github.com/hmans/miniplex/blob/main/packages/miniplex/src/World.ts#L183

The options are here:

  1. Ignore and continue on
  2. Replace the existing component
  3. Throw an error

It might be correct to throw an error in this case because your entity could get into an unexpected state if you weren't expecting there to be a component there already.

So maybe for sake of clarity in the addition its best to keep it, but remove the error for the remove case?

hmans commented

You are right on the money with this, it has been one of my least favorite bits in Miniplex. After some user feedback that made it clear to me that the ECS-like strictness Miniplex was holding on to was causing various problems (from being cumbersome to, argh!, making it really hard to make the damn thing work in React's StrictMode), I started working on a major new version this week that I will be releasing as 2.0 soon. It'll be both more relaxed, eschewing some of the ECSisms for a more relaxed they're-just-objects approach, but also more flexible and significantly faster, too.

I don't want to make major changes like this to 1.0 at this point, and would prefer to get 2.0 out as quickly as I can to have people use that instead. (If you're curious, I've been pushing pre-release builds, but the documentation has not been updated yet.)

hmans commented

If you want to brief pre-documentation overview of what's coming in 2.0, please check out this issue: #165

Cool thanks, ill close this and comment over there then :)