New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving classes into folders #194

Closed
WingEraser opened this Issue Nov 29, 2013 · 22 comments

Comments

Projects
None yet
5 participants
@WingEraser

WingEraser commented Nov 29, 2013

@IQAndreas

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas Nov 29, 2013

Member

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.

Member

IQAndreas commented Nov 29, 2013

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

This comment has been minimized.

Show comment
Hide comment
@WingEraser

WingEraser Nov 29, 2013

I see your comments ;)

WingEraser commented Nov 29, 2013

I see your comments ;)

@flunardelli

This comment has been minimized.

Show comment
Hide comment
@flunardelli

flunardelli Nov 30, 2013

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.

flunardelli commented Nov 30, 2013

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

This comment has been minimized.

Show comment
Hide comment
@WingEraser

WingEraser Nov 30, 2013

Just looked inside haxeflixel

  • Will there be more classes for FlxGroup? Then a group folder could be added
  • 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 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

WingEraser commented Nov 30, 2013

Just looked inside haxeflixel

  • Will there be more classes for FlxGroup? Then a group folder could be added
  • 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 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

This comment has been minimized.

Show comment
Hide comment
@Dovyski

Dovyski Nov 30, 2013

Member

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.

Member

Dovyski commented Nov 30, 2013

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

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas Apr 27, 2014

Member

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.

Member

IQAndreas commented Apr 27, 2014

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

This comment has been minimized.

Show comment
Hide comment
@FlixelCommunityBot

FlixelCommunityBot Apr 27, 2014

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.

FlixelCommunityBot commented Apr 27, 2014

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

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas Apr 27, 2014

Member

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):

Member

IQAndreas commented Apr 27, 2014

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

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas Apr 28, 2014

Member

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?

Member

IQAndreas commented Apr 28, 2014

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

This comment has been minimized.

Show comment
Hide comment
@Dovyski

Dovyski Apr 28, 2014

Member

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?

Member

Dovyski commented Apr 28, 2014

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

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas Apr 28, 2014

Member

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.

Member

IQAndreas commented Apr 28, 2014

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

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas Apr 28, 2014

Member

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?

Member

IQAndreas commented Apr 28, 2014

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

This comment has been minimized.

Show comment
Hide comment
@Dovyski

Dovyski Apr 30, 2014

Member

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.

Member

Dovyski commented Apr 30, 2014

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

This comment has been minimized.

Show comment
Hide comment
@Dovyski

Dovyski Apr 30, 2014

Member

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

  • Move FlxSprite to root
  • Move FlxAnim to animation

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

Member

Dovyski commented Apr 30, 2014

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

  • Move FlxSprite to root
  • Move FlxAnim to animation

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

@IQAndreas

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas Apr 30, 2014

Member

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.

Member

IQAndreas commented Apr 30, 2014

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

This comment has been minimized.

Show comment
Hide comment
@Dovyski

Dovyski Apr 30, 2014

Member

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

Member

Dovyski commented Apr 30, 2014

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

@IQAndreas

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas May 1, 2014

Member

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. ;)

Member

IQAndreas commented May 1, 2014

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

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas May 1, 2014

Member

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.

Member

IQAndreas commented May 1, 2014

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

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas May 1, 2014

Member

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?

Member

IQAndreas commented May 1, 2014

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

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas May 1, 2014

Member

I added a pre-emptive pull request for this issue: #205

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

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.

Member

IQAndreas commented May 1, 2014

I added a pre-emptive pull request for this issue: #205

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

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

This comment has been minimized.

Show comment
Hide comment
@Dovyski

Dovyski May 2, 2014

Member

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.

Member

Dovyski commented May 2, 2014

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 added a commit that referenced this issue May 3, 2014

Merge pull request #205 from IQAndreas/organize-flixel
Organize Flixel into packages

Fixes #194

* Change main package from `org.flixel` to just `flixel`
* Organize Flixel's classes into several, more logically organized packages
* Rename `FlxAnim` to `FlxAnimation`
@IQAndreas

This comment has been minimized.

Show comment
Hide comment
@IQAndreas

IQAndreas May 3, 2014

Member

Merged in ca946ab

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

Member

IQAndreas commented May 3, 2014

Merged in ca946ab

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

@IQAndreas IQAndreas closed this May 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment