Skip to content
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

Auto detect window size and etc. #1811

Merged
merged 22 commits into from
Apr 11, 2016
Merged

Auto detect window size and etc. #1811

merged 22 commits into from
Apr 11, 2016

Conversation

buckle2000
Copy link
Contributor

Tested. works. see here

@buckle2000
Copy link
Contributor Author

Also, please change the template of Main.hx (Main.hx.tpl) into a static file (Main.hx).
${WIDTH} and ${WIDTH} can be simply replaced with 0.

@buckle2000 buckle2000 changed the title Auto detect window size. Auto detect window size and etc. Apr 11, 2016
@buckle2000
Copy link
Contributor Author

Added function multiply to FlxPoint.
It might be useful.
e.g.

sprite.scale.multiply(2.0)

Also, try operators overloading?

* @param scalar How much times this point will be as it before.
* @return This point.
*/
public function multiply(scalar:Float):FlxPoint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FlxVector#scale() is the same thing, so this seems redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not redundant.
Some of HaxeFlixel's code use FlxPoint instead of FlxVector.
So you have to write code like this:

point.x*=scalar;
point.y*=scalar;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware, that doesn't make it any less redundant - both functions have the same implementation. Since FlxVector extends FlxPoint, FlxVector would have a scale() and a multiply().

If anything, scale() should be moved to FlxPoint().

@@ -243,6 +243,14 @@ class FlxGame extends Sprite
// Super high priority init stuff
_inputContainer = new Sprite();

if (GameSizeX == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The special meaning of 0 for GameSizeX and GameSizeY should be documented in the @param docs.

@Gama11 Gama11 added this to the 4.1.0 milestone Apr 11, 2016
@Gama11
Copy link
Member

Gama11 commented Apr 11, 2016

Also, try operators overloading?

Operator overloading only works for abstracts. I've experimented with it a bit once... You would need a FlxPoint abstract and a FlxPointImpl class or something of the sort. I think I had some issues with methods that return FlxPoint instances / this, but I don't remember why exactly.

Operator overloading might be supported via static extensions in Haxe 4 (HaxeFoundation/haxe#2803), which would be quite convenient. Of course, that's not gonna be available anytime soon.

* @param k - scale coefficient
* @return scaled vector
*/
public inline function scale(k:Float):FlxVector
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still returns a FlxVector and says @return scaled vector.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this is going to result in sprite.scale.scale(), which looks quite weird... Oh well.

* @param k - scale coefficient
* @return scaled vector
*/
public inline override function scale(k:Float):FlxVector
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An inlined function can't be overriden. Are you actually compiling this code locally? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no. both with override or without override failed.
I tried this:

class A{
    public static function main(){
        new B().a();
    }
}

class B extends C {
    public **override** inline function a():B {
        return this;
    }
}

class C {
    public inline function a():C {
        return this;
    }
}

with override:

Field a is inlined and cannot be overridden

without:

Field a should be declared with 'override' since it is inherited from superclass C

wtf

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that FlxPoint#scale() is inline - you can't override a function that is inline, otherwise the compiler wouldn't be able to inline it anymore.

You definitely can't omit the override keyword.

@@ -223,16 +223,16 @@ class FlxGame extends Sprite
/**
* Instantiate a new game object.
*
* @param GameSizeX The width of your game in game pixels, not necessarily final display pixels (see Zoom).
* @param GameSizeY The height of your game in game pixels, not necessarily final display pixels (see Zoom).
* @param GameSizeX The width of your game in game pixels, not necessarily final display pixels (see Zoom). If it is less than 0, HaxeFlixel will auto detect the width according to the current window.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These docs are not quite accurate, they only account for the < 0 case, not == 0.

@buckle2000
Copy link
Contributor Author

Also, template change.

@@ -243,6 +243,14 @@ class FlxGame extends Sprite
// Super high priority init stuff
_inputContainer = new Sprite();

if (GameSizeX <= 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should stick to only == 0. Something like new FlxGame(-10, -50, PlayState) would result in windowSize x windowHeight, which seems a little weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't understand what do you mean by windowSize x windowHeight.
Another reason I can think of is to reserve the negative values for possible more implements in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant windowWidth x windowHeight.

What I meant is that it seems confusing to the reader that random negative values would turn into a FlxGame instance which uses the window dimensions, wheareas with 0, you can at least guess that it probably has some special meaning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the comments, so it is easier to be understood.
Just think if somehow a negative value is passed in, FlxGame can deal with it.

@buckle2000
Copy link
Contributor Author

just keep both multiply and scale.

@Gama11
Copy link
Member

Gama11 commented Apr 11, 2016

Why? I don't agree with that. Having two functions in FlxVector that do the same thing is bad API design.

@@ -243,6 +243,14 @@ class FlxGame extends Sprite
// Super high priority init stuff
_inputContainer = new Sprite();

if (GameSizeX <= 0)
{
GameSizeX = Std.int(Lib.current.stage.window.width);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't use .window here, needs to be stageWidth and stageHeight (Travis is currently failing because Stage#window doesn't exist in OpenFL legacy).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried using Lib.current.stage.width but not Lib.current.stage.*window*.width.
BUT, Lib.current.stage.width IS 0! I don't know why.
I figured that out when targeting flash.
Oh, also it is a reason not to use FlxG.stage.width.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stage.stageWidth, not stage.width.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I thought they are the same.
stage.stageWidth does work.

@buckle2000
Copy link
Contributor Author

Well, if not, how to solve the "inline" problem?
remove inline?
use some dark magic of haxe to make multiiply un-accessible in FlxVector?
or just forget about that multiply?

@Gama11
Copy link
Member

Gama11 commented Apr 11, 2016

Yeah, just remove the inline. It's only a minor optimization.

@buckle2000
Copy link
Contributor Author

OK.
Another question:
as you mentioned:

Having two functions in FlxVector that do the same thing is bad API design.

What about FlxVector.clone and FlxPoint.copyTo?

@Gama11
Copy link
Member

Gama11 commented Apr 11, 2016

Yeah, not a fan of that.

@Gama11 Gama11 merged commit 5f271eb into HaxeFlixel:dev Apr 11, 2016
Gama11 added a commit that referenced this pull request Apr 11, 2016
Gama11 pushed a commit to HaxeFlixel/flixel-templates that referenced this pull request Apr 11, 2016
Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants