Author Topic: nitpicks...  (Read 3680 times)

shaktool

  • Guest
nitpicks...
« on: Fri, Jun 5, 2009 »
Because I care.  ;)

FlxCore and FlxState frequently use the keyword "virtual", which in AS3 is defined to do precisely nothing. If you find it useful to mark which functions you intended to override elsewhere, that's cool, but it's unnecessary and assumed.

In FlxCore, if I'm reading your comments correctly, the words "alive" and "dead" are not simply opposites of each other, which is confusing. Why not rename "dead" to "ethereal", or "intangible", or better yet, flip it and call it "tangible". If you still want to allow people to directly query whether something is dead, throw in a getter called "dead" that simply negates and returns the "alive" property.

In FlxCore, scrollFactor is described as a Number when in fact it is a Point. This should be clarified.

In FlxCore, flickering() and onScreen() should intuitively be getters.

In FlxAnim and FlxSprite.addAnimation(), what's with the SingleFrame parameter? Is it supposed to be more intuitive than simply passing a frame array of length one? (e.g. [4])

In FlxAnim and FlxSprite.addAnimation(), the FrameRate parameter is undocumented.

While reading about the private "loopMusic" function in FlxG, I had flashbacks to when I was trying to figure out why Flash was looping my music incorrectly. I ended up doing pretty much the same thing you did, but Flash's behavior here still boggles my mind. :P

In FlxG, all of the "internal management" functions should be marked as "internal" rather than "public". This gives other classes in the same package/directory access to it, but not classes in different packages.

I think the terminology of the "nonexist" feature needs to be cleaned up. I think "empty" might be a better word, because it implies that there is at least a container for something to be there, whereas "nonexistant" means the same thing to me as "null", even though the words are used to mean entirely different things here. Similarly, the usage of "remove" and "forceDelete" in flxArray bother me. Removing an element from array should not mean setting a property on an element inside the array.

In FlxArray.removeAt(), AFAIK the "delete" line is a redundant and misleading version of the "= null" line. Kill it.

FlxArray.cleanup() should probably be implemented as:
length = 0;

In FlxBlock, I'd replace the "Empties" parameter with a number from 0 to 1 indicating the probability of any given tile being empty. First of all, this is a more accurate model of what's going on already if I understand it correctly, since the actual number of empty tiles is somewhat random, and second of all, this means that I can adjust the width and height and keep the probability the same and it will give me the same density of empty tiles.

In FlxButton, on() should be a getter. And again, update() should be "internal" rather than "public".

It would be nice if FlxSprite.overlapArrays(), or a similar function, could be used to collide an array of sprites with itself, but without accidentally colliding each sprite with itself. Meaning, collide each sprite with all of the OTHER sprites in the array.
« Last Edit: Fri, Jun 5, 2009 by shaktool »

shaktool

  • Guest
Re: nitpicks...
« Reply #1 on: Fri, Jun 5, 2009 »
Oh hey, and it looks like you're implementing double buffering? You're not the first indie game developer I've caught doing that in Flash.  ::) Flash is double buffered already, doing it with your own bitmaps is redundant. Except for bizarre circumstances (an error being thrown and not caught), the screen will not be updated until you return from your enterFrame function.
« Last Edit: Fri, Jun 5, 2009 by shaktool »

Adam Atomic

  • Founder
  • Key Contributor
  • *****
  • Posts: 852
  • Karma: +0/-0
  • new dad
    • View Profile
    • Adam Atomic
Re: nitpicks...
« Reply #2 on: Sat, Jun 6, 2009 »
Hey John thanks for the checkup!

Double Buffering - Ok this actually doesn't surprise me because I get tearing and stuff even though I've "manually" double-buffered it, I am definitely happy to tear that stuff out...

Defining virtual - Noted, I do think its useful though just for my own personal organization

Alive vs Dead - Yea the only reason I went with "dead" is its very useful way of checking if you've killed off an entity but want to continue to animate it or leave the corpse in the level.  Really "Alive" is the more problematic of the variable names, as it is merely a toggle for whether or not the update() function should be called by the game loop at all.  It'd probably be a relatively simple change but I'm not sure what would be a better name for it yet!

scrollfactor - ah **** yea that needs to be defined as a point (it USED to be a number :P )

getters - I am totally freaked out by setters and getters :P  There are a few places in the code that should be setters and getters though (See the volume controls for the sound subsystem in FlxG for much worse example than flickering and onscreen :D ) so I'll try and get those into the next public too

SingleFrame - aarrgghh Flash man.  I tell you what.  So...when you pass flash arrays a single parameter in their constructor, it does not create an array with a single parameter (if it is numbers).  E.g. [4] would create an array with 4 empty slots, not an array with just he number 4 in it.  Not sure about the best way to handle this weirdness still.  Ideally you could pass it a FlxArray but I can't figure out how to make a constructor for FlxArray that takes sort of infinite parameters (because I could then define FlxArray to NOT create a bunch of empties)

FrameRate - woops I should fix that!

loopmusic - yea i hate it.  has to do with the way the position parameter interacts with the loop parameter of the Sound.play() function.  If you pass a starting position AND a number of loops (like 10000 or whatever) it performs each loop from the same starting position.  it's so F'ing retarded.

internal - YES i totally need to do that, had no idea it existed

empty/nonexist/etc - Yeah I'm not sure if the terminology and array functionality here is completely ideal.  Some of this is left over from when I was actually deleting things and checking for null, instead of using a FlxCore with a "exists" flag inside these big automated loops.  I am not sure what the best variable names are yet either, but I think 'exists' is pretty much not THAT bad...but some of the array stuff doesn't make much sense anymore.

The other stuff looks right but I have to run!  anyways thanks for all the feedback :D

(by looks right i mean you are right not flixel :D  )

shaktool

  • Guest
Re: nitpicks...
« Reply #3 on: Sat, Jun 6, 2009 »
 ;D

SingleFrame - aarrgghh Flash man.  I tell you what.  So...when you pass flash arrays a single parameter in their constructor, it does not create an array with a single parameter (if it is numbers).  E.g. [4] would create an array with 4 empty slots, not an array with just he number 4 in it.  Not sure about the best way to handle this weirdness still.  Ideally you could pass it a FlxArray but I can't figure out how to make a constructor for FlxArray that takes sort of infinite parameters (because I could then define FlxArray to NOT create a bunch of empties)

I'm pretty sure if you say "new Array(4)" it will give you an array with 4 elements, but if you use the literal array constuctor "[4]" it will give you an array containing the number four.

Ipsquiggle

  • Guest
Re: nitpicks...
« Reply #4 on: Sat, Jun 6, 2009 »
Double-buffering... I see tearing in flash all the freakin' time. Are you sure it double-buffers by default? I'd do some tests before ripping that out.

Passing in a variable number of parameters:
http://livedocs.adobe.com/flex/2/langref/statements.html#..._(rest)_parameter

Alive vs. Dead: It seems to me you should just be literal about it: Call them "updates" and "collides" or something like that. This is more in line with "visible".

All that said, I'm now going to start USING this library. I'll chime in with more detail later... Thanks! :D

Melly

  • Member
  • **
  • Posts: 24
  • Karma: +0/-0
    • View Profile
Re: nitpicks...
« Reply #5 on: Sun, Jun 7, 2009 »
I think the main reason for Flash's tearing is not double-buffering. The point of double-buffering is to make sure you don't have sprites popping in and out of existence between frames, or flickering. It does nothing about tearing, because tearing is related to vertical synchronization, or vsync, which flash doesn't have. Without vsync it's near-impossible to avoid tearing on most computers, so don't even bother.

Melly

  • Member
  • **
  • Posts: 24
  • Karma: +0/-0
    • View Profile
Re: nitpicks...
« Reply #6 on: Sun, Jun 7, 2009 »
I also agree with most of the points raised by John Nesky. Some things I'd like to add:

You use Math.floor() instead of int() in places where it'd work much better, like in the FlxArray.getRandom() function.

  • I've heard, and tested to some extent, that uint's are much slower in AS3 than int's, at least at the version I tested. I'll get the latest version of Flex and test it again, just to be certain, but I think the consensus is that you only use uint when necessary and int for everything that doesn't need decimals, especially 'for' loops.

Tyler Egeto

  • Guest
Re: nitpicks...
« Reply #7 on: Sun, Jun 7, 2009 »
I've heard, and tested to some extent, that uint's are much slower in AS3 than int's, at least at the version I tested. I'll get the latest version of Flex and test it again, just to be certain, but I think the consensus is that you only use uint when necessary and int for everything that doesn't need decimals, especially 'for' loops.

This stuff is very situational, uint's can be slower or faster, for loops (all loops) do work fastest with ints. Any math that results in decimals is faster with Numbers (int/uint has to be converted internally) These situations change with the player versions though, for example in player 10 int/uint/number conversions are a lot faster than player 9. It's