breaking apart IgeEntity

A place to request and discuss new engine features / functionality

breaking apart IgeEntity

Postby mrleerman » Wed Feb 26, 2014 5:43 am

This is less of a new feature and more of a refactor. I was attempting to put in a custom rendering order when I discovered the update() and tick() functions in IgeEntity. Which lead to my noticing the rather large amount of code within IgeEntity. Glancing through what's there, it is clear that the contents of IgeEntity could be broken into multiple components to better decouple its responsibilities. To that end, I have begun the effort by creating IgeEntityStreamComponent to house all of the streaming code from IgeEntity.

Rob, would you be interested in taking this change into the engine? It'd be great if I didn't have to cross file merge any future changes to IgeEntity's streaming code into my changes ;)
mrleerman
 
Posts: 2
Joined: Mon Feb 24, 2014 6:47 am
Location: Atlanta, GA

Re: breaking apart IgeEntity

Postby robaldred » Fri Feb 28, 2014 11:47 am

Hey, feel free to post up a link to your branch, I'm sure others would be happy to comment and collaborate.
I don't work for Irrelon. All comments are my own.
I would strongly encourage anyone building production projects with Isogenic to buy a premium licence, it costs very little and will help towards continued development of the engine. Irrelon have spent thousands of hours developing this wonderful platform with many features found in AAA game engine.
User avatar
robaldred
 
Posts: 243
Joined: Wed Oct 23, 2013 8:09 pm
Location: Manchester, England

Re: breaking apart IgeEntity

Postby Doidel » Fri Feb 28, 2014 4:10 pm

Just decoupling the streaming from the entity will break existing code and won't be backwards compatible.
You could maybe "addComponent" the stream component in the init of IgeEntity. But then still every entity would posess the streaming functionalities and besides some code refactoring nothing will have changed.
Further since a major revamp of the streaming system is in progress I would currently leave such a refactor aside. In the worst case it would just complicate things.

I'd maybe await Rob's opinion on this. Thanks for the suggestion though!
User avatar
Doidel
 
Posts: 59
Joined: Tue Jan 14, 2014 5:05 pm

Re: breaking apart IgeEntity

Postby mrleerman » Sun Mar 02, 2014 1:06 am

I'm nearly done testing my changes against the example projects (a few of which were apparently already broken). Once I'm done with that I'll submit my changes up into the entity-refactor branch.

I've done precisely what you mentioned Doidel; I added the new component to all IgeEntitys via addComponent during their init. I disagree that this changes nothing. At the very least it makes IgeEntity.js a little more legible and maintainable. In the long term, I think it would be preferable if Entities were not required to have the stream (or any) component. In my view, slicing up the IgeEntity monolith is merely step one in a sensible path to that goal.

Also, I noticed the streaming refactor you've been working on and am pretty sure it'll be fairly easy to integrate my changes. I assumed the burden of that integration would fall to me since you've already submitted a pull request.

Edit: If anyone's curious, it appears that examples 13.7, 24.3 and 24.5 are broken. At cursory glance, it appears that pathing API changes and the THREE library are at fault. Also, I noticed that turning on grid display for an IgeTextureMap that isn't auto sectioned results in the grid and the texture map being drawn offset from one another. I'll see about submitting actual bugs / fixes for these later.

Edit edit: Btw yes, Doidel is quite right: This is a breaking change. I was pretty sure the dev branch already contained breaking changes so this seems like an opportune moment.
mrleerman
 
Posts: 2
Joined: Mon Feb 24, 2014 6:47 am
Location: Atlanta, GA

Re: breaking apart IgeEntity

Postby Doidel » Sun Mar 02, 2014 11:45 am

Well, sure, with the stream stuff as component, added in the init, no harm's done. That change is very much welcome, and thanks for the pull request :)

Unfortunately since some examples / public code assign streamModes and rooms before the stream component is initialized I currently don't see a non-breaking way of adding the entities' stream component only when needed...

I plan to complete my rework within the next week (no promises). If you could afterwards make your slice-up work with my changes I can offer to test your change on all the test-cases I've gathered so far :)

However the thing you do is no breaking change to my knowledge. And neither is any of the code I am working on - the whole stream system revamp works with existing code (or is at least planned to do so).
User avatar
Doidel
 
Posts: 59
Joined: Tue Jan 14, 2014 5:05 pm


Return to New Feature Discussion

Who is online

Users browsing this forum: No registered users and 1 guest
cron