onehundredfeet / hmecs

Super lightweight Entity Component System framework for Haxe
MIT License
14 stars 2 forks source link

Optimize `entity.add()`. #1

Closed player-03 closed 1 year ago

player-03 commented 3 years ago

Instead of attempting to add an entity to ALL views, it's best to check only views of the newly-added component. So if you add a String component, the function will call ViewsOfComponentString.inst().addIfMatched() instead of iterating through Workflow.views.

Interestingly, entity.remove() was already optimized this way, so I assume this was an oversight.

player-03 commented 3 years ago

Also, any chance you could turn on Issues for this repo? I have questions/comments, but there's nowhere to put them.

It'll be on the settings page, under "features."

onehundredfeet commented 2 years ago

Sorry, I totally missed this. I'll turn on issues.

onehundredfeet commented 2 years ago

I added the features. I will look at your PR.

I also just renamed the depot. Sorry if that causes you any issues.

onehundredfeet commented 2 years ago

Would you mind resolving against the changes? There's been so many it's hard to see through it.

player-03 commented 2 years ago

Finally done, I think.

player-03 commented 1 year ago

Ok, but for the record, the issue is still very much present.

macro if (__entity__.isActive()) {
    for (v in ecs.Workflow.views) { // <- inefficient
        @:privateAccess v.addIfMatched(__entity__);
    }
}

You're iterating through all views each time you add any component, despite the fact that most of those views don't even include that component and therefore couldn't possibly be affected.

onehundredfeet commented 1 year ago

Agreed. Nice to hear from you again.

I haven't had a lot of time to look at this but I've come into some more time now and was going to take a look.

I'm not sure why it said I closed the issue? I didn't touch the issue status.

player-03 commented 1 year ago

Oh, ok. I'll put it back then.

Whatever happened, it prompted me to look at your latest change, which I'm actually considering using. So that's good.

onehundredfeet commented 1 year ago

huh? why is it auto closing?

onehundredfeet commented 1 year ago

Reopening.

onehundredfeet commented 1 year ago

All the changes disappeared, what is going on?

player-03 commented 1 year ago

I'll open a new one.

onehundredfeet commented 1 year ago

I was browsing your branch. You've done an impressive amount of updates.

player-03 commented 1 year ago

Thanks. Hope you can find some useful stuff in there. And I'm happy to explain if you find anything confusing.

player-03 commented 1 year ago

Betting it broke because player-03/echoes is no longer a fork. So I sent the new pull request from my forked repo instead.