FlixelCommunity / flixel

Community fork of Adam “Atomic” Saltsman's popular game engine Flixel. Distilled from a variety of Flash games he worked on over the last couple years, including Gravity Hook, Fathom and Canabalt, its primary function is to provide some useful base classes that you can extend to make your own game objects.
http://flixelcommunity.org/
Other
84 stars 17 forks source link

Moving classes into folders #194

Closed WingEraser closed 10 years ago

WingEraser commented 10 years ago

Check the spreadsheet. It's public, so edit as you like.

https://docs.google.com/spreadsheet/ccc?key=0AnxnwZ_0WkxsdENoanBtYXUybk5rVEpzUnpGcFo1NkE&usp=sharing

IQAndreas commented 10 years ago

I started moving stuff, but then switched to adding comments instead of actually moving anything. i think it may be better that way since you guys can see the reasoning behind why I feel something should be moved elsewhere, and you can write whether you agree or disagree with my suggestions.

I'm rather new to using Google docs, so I'm not sure if you guys can see the comments I made, or if they were only saved to my own copy of the spreadsheet.

WingEraser commented 10 years ago

I see your comments ;)

flunardelli commented 10 years ago

I would like to suggest to look for ideas in haxeflixel project. I known that is a different beast but it seems very concise.

On 29/11/2013, at 17:15, Ka Wing Chin notifications@github.com wrote:

I see your comments ;)

— Reply to this email directly or view it on GitHub.

WingEraser commented 10 years ago

Just looked inside haxeflixel

I haven't added plugin folder, because I think a plugin should work without putting inside the core. edit: lol, I do have added plugin, because of DebugPathDisplay and TimerManager. A manager folder perhaps and move DebugPathDisplay to debug

Dovyski commented 10 years ago

I've just added my 2 cents in the comments at the spreadsheet.

Will there be more classes for FlxGroup? Then a group folder could be added

I don't think we are going to add more classes for FlxGroup, so there is no need to create a group package.

The effects folder got emitter, particle, trail. Will there be more Camera effects for flixel in the future? And those effects at FPT, will they be added, then effects folder could be added.

I don't know if FPT's effects will be added, but I like the idea of an effects package containing emitter and particle.

I haven't added plugin folder, because I think a plugin should work without putting inside the core. edit: lol, I do have added plugin, because of DebugPathDisplay and TimerManager. A manager folder perhaps and move DebugPathDisplay to debug

I guess that plugin folder is quite useful. When we improve the plugin system (#121), a folder to house all new classes will be nice.

IQAndreas commented 10 years ago

I originally posted this comment in the wrong area, so I will move it here.

Any updates on this issue, @IQAndreas ?

Well, before I "went AWOL", I was looking into the source of HaXe Flixel; I knew they had organized the classes into packages, and was hoping we could mirror their actions as closely as possible (making tutorials written for HaXe Flixel work in AS3 Flixel as well, or make it easier for users of one to seamlessly switch over to using the other).

I was under the impression that HaXe Flixel was still in progress, or an early beta. However, upon seeing the code, most of the AS3 classes seem to all have been converted, and not only that, but there were many new classes added. See: https://groups.google.com/forum/#!topic/haxeflixel/N28HHhXffwM

I haven't had the opportunity to compare the HaXe Flixel package structure to our suggestions listed on Google Docs, however, I would like to do so before proceeding. Otherwise, it looks like we have mostly ironed out where everything should go.

FlixelCommunityBot commented 10 years ago

Comment by Dovyski April 11, 2014 at 05:27:55-07:00


Yeah, HaxeFlixel is a mature project. I've seen several games made with it, including some for OUYA. @Gama11 suggestion is a good way to go; I don't believe our discussed package structure differs that much from the Haxe version.

IQAndreas commented 10 years ago

I don't believe our discussed package structure differs that much from the Haxe version.

I did a quick ls -R (man, I love Bash) on the HaxeFlixel source folder, and came up with these results (moved to Gist since it was taking up space as a long comment):

IQAndreas commented 10 years ago

I "colored" and commented the original spreadsheet in such a way that it is clear which classes share the same package as their HaxeFlixel counterparts:

I also branched the spreadsheet off to a second document where I did a small amount of changes that I feel are the most important:

There are more green cells on that one, yet the ones that don't match, I feel don't match for a good reason. Other than that, the changes are not huge.

Any thoughts, agreements, or disagreements?

Dovyski commented 10 years ago

Awesome work, @IQAndreas ! I've made several comments on the original spreadsheet with my point of view. Some comments suggest a few changes that you didn't do when you created a second spreadsheet.

Should be discuss your spreadsheet from now on, working from there?

IQAndreas commented 10 years ago

Should be discuss your spreadsheet from now on, working from there?

Either one is fine by me. I just created the second spreadsheet because I didn't want to interfere with the original one if we ended up reverting most of the changes. It's a shame the comments didn't follow along with it.

If you feel most of the changes will be kept in the second spreadsheet, we can start commenting there instead, since most of the old comments are now applied or obsolete in the new one.

IQAndreas commented 10 years ago

I put the HaxeFlixel folder structure in a spreadsheet for easier access:

One thing to note is there are several classes that only have one class of that type in Flixel (for instance, FlxAnim and FlxText), but have several classes in that category in HaxeFlixel (For animation, FlxAnimationController, FlxAnimation, FlxBaseAnimation, and FlxPrerotatedAnimation, and then for text, FlxBitmapTextField, FlxTextField, FlxText, etc.). In that case, it would make sense that HaxeFlixel has its own package to contain all those similar classes, but it seems odd having it's own package for just a single Flixel class.

Is it better to keep classes grouped in larger packages (for instance, FlxAnim in display), and then move it into its own package (animation) once it gains more "similar classes"? Or is it better to pre-emptively keep it in the animation package, so that existing code won't break once the classes are moved in the future?

Dovyski commented 10 years ago

It's better to pre-emptively move classes in packages now, even if a package will have a single class. It will save _a lot_ of headache in the future. I faced that very same question a few years ago and back then we decided to use larger packages, moving classes later. It was hell.

Dovyski commented 10 years ago

The structure in Flixel folder structure - Version 2 seems good to me, except for:

After those changes, I'm ok with everything :)

IQAndreas commented 10 years ago

Agree with both of those. There are three last items I added comments on, (just because I brought it it up doesn't necessarily mean I agree with it, just thought I'd add those points in for completeness sake).

After that, I'm satisfied with how it looks, and I'm surprised at how well we were able to match HaxeFlixel's folder structure; this will help a lot of people who are switching from AS3 to Haxe, or working with both simultaneously, as well as tutorials written for either language.

Dovyski commented 10 years ago

Yeah, that's true. It will help foster both projects.

IQAndreas commented 10 years ago

If everyone is satisfied with how the spreadsheet looks, does anyone mind if I get started on the pull request? (It's listed as belonging to @WingEraser, but you are welcome to steal one of my issues as revenge. ;)

IQAndreas commented 10 years ago

One thing I noticed, there are two subfolders inside of data (which now gets renamed to assets) for vcr and vis. Should those remain in the assets folder in their respective sub-folders, or do we want them in system/debug or system/debug/assets along with the classes that use them?

I prefer the former, but I'm fine with either way.

IQAndreas commented 10 years ago

One more thing, HaxeFlixel puts the project in a package flixel, whereas Flixel originally (and currently) uses org.flixel.

Since we have "diverged" from flixel.org, and it seems tacky towards Adam renaming the package org.flixelcommunity, shall we follow HaxeFlixel's pattern and use just the flixel package as well?

IQAndreas commented 10 years ago

I added a pre-emptive pull request for this issue: https://github.com/FlixelCommunity/flixel/pull/205

I made some decisions on the questions I posed earlier:

But if anyone disagrees with those decisions, changing them is only a commit away, which I will gladly do.

As mentioned in the pull requests, there are still two decisions that need to be made (FlxRect and FlxList); those decisions I was not comfortable with making without checking with you guys first, so I'll leave it up to you guys to decide, and then add those commits to the pull request.

Dovyski commented 10 years ago

I made some decisions on the questions I posed earlier:

  • Used assets/vcr instead of system/debug/assets/vcr
  • Used the flixel global package instead of org.flixel

Perfect! I commented on the pull request about the renames. It's better to match HaxeFlixel names, so I think we should use FlxRect and FlxList.

IQAndreas commented 10 years ago

Merged in ca946ab5b2a60ca7d4c1df13e84d29fb2fb40c01

By the way, that was a great idea with the spreadsheet, @WingEraser, it really helped keep things organized and clear.