craftyjs / Crafty

JavaScript Game Engine
http://craftyjs.com
MIT License
3.42k stars 557 forks source link

Logic separation of components #578

Open potomak opened 11 years ago

potomak commented 11 years ago

I think that separation of concerns in components is great and it just works, but I'd like to see only one component per file. Directories could be used to aggregate cohesive components.

See also #577.

dariusk commented 11 years ago

I agree completely.

starwed commented 11 years ago

I'd like to see only one component per file.

Hmm, I'm not so certain about this as a hard rule. Some components are very short, and depend heavily on other components. For instance, should "FourWay" be in a separate file from "MultiWay"? All the former component does is call multiway. Having to constantly hop between files hurts readability just as much as having to navigate a huge monolithic file.

There are scores of components that should be in their own files, though, so I agree in general. And I think everything in its own file, while not ideal, is certainly an improvement over the current situation.

kevinsimper commented 11 years ago

I think the problem is in the files which is over 700 lines, which is the 2D.js, controls.js, core.js, math.js files.

And as you point out, it does not make any sense to split out FourWay and MultiWay, but i do not think that was what @potomak was thinking either :)

potomak commented 11 years ago

@starwed @kevinsimper in my opinion there should be one clear rule: one file, one components.

A simple solution to the *way components problem should be to put them in the same directory (package), maybe the controls directory.

starwed commented 11 years ago

A simple solution to the *way compoents problem should be to put them in the same directory (package), maybe the controls directory.

That's not a solution to the objection I had; in fact, putting the source code in subdirectories can actually make it harder to jump between files, depending on what sort of development environment you use.

potomak commented 11 years ago

Note: one of the main reasons (a practical one) I'm asking for this feature is that when I'm looking for a component I'm always forced to look for the file including it using the search in project function of my editor.

potomak commented 11 years ago

Putting more than one component per file is like putting more than one class per file, it's permitted in some languages. I write mostly ruby and java and it just feels wrong to me.

I challenge you @starwed to put all *way components api documentation together in one single doc page as they're in a single js file :smile:

starwed commented 11 years ago

In the small amount of time I spent with Java I hated exactly that, so I think there's a large subjective component to it! :P

But clearly the current state is not great. Why don't we start by identifying files that hold multiple, unrelated components.

One very basic separation we should consider is between code that depends on a browser context, and code that doesn't. We should make it easy for someone to use Crafty code on the backend, as described in #440.

mucaho commented 11 years ago

If you give me the o.k. to @starwed idea to seperate browser context from rest of code, I can give it a try. If not much has changed since 0.5.3, then the seperation should be pretty simple as described in backend differences in v0.5.3

starwed commented 9 years ago

I've gone through recently and drastically improved the organization, but there's still lots of room for improvement!

mucaho commented 9 years ago

@starwed I suggest we also organize the tests into the same folder/file structure as their corresponding source files.

starwed commented 9 years ago

@mucaho Would we then split tests per the file they correspond to? That might be a good way to see where we're missing test coverage. Would require a bit of busy work, of course. :)

An intermediate step if we don't want that level of granularity yet would just be to have a test file corresponding to each folder.

mucaho commented 9 years ago

An intermediate step if we don't want that level of granularity yet would just be to have a test file corresponding to each folder.

That sounds like a good alternative - one test file per source folder, which is further separated into test modules corresponding to each source file. We can clearly see test coverage and avoid sparsely populated test folders.
However, that will limit our options in the long run, since not all test files inside a folder can be executed in a browser-less (node) environment. In this case it's better to have separate test files (unless there is an option to run specific test modules only).

mucaho commented 9 years ago

Additional thoughts/suggestions:

starwed commented 9 years ago
mucaho commented 9 years ago

I'm not sure I see much point in splitting it further -- you'd have things that could apply to any 2D object, and then things that only matter if it's being rendered.

Sounds good. My primary concern is about maintainability - it's a bit difficult to see everything that gets triggered when you modify an entity's coordinate. That's why splitting it even further (the collision stuff, the propagation to children, etc...) could maybe also make sense.

mucaho commented 9 years ago

@starwed Also, what do you think about splitting the "DOM" layer and components into "DOM"(2D transforms) and "DOM3D" (3D transforms) - then it would be similar to "Canvas" and "WebGL". If you're worried about browser compatibility you would use "DOM" and "Canvas". If you want state-of-the art drawing you would use "DOM3" and "WebGL".

mucaho commented 8 years ago

Recent mouse suggestions got me thinking:

Also, the events for or triggered by these (hopefully soon-to-be systems) would then be localized to each system, rather than triggered globally on the Crafty object.