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

Hscript console, closes #1156 #1637

Merged
merged 8 commits into from
Nov 27, 2015
Merged

Hscript console, closes #1156 #1637

merged 8 commits into from
Nov 27, 2015

Conversation

MSGhero
Copy link
Member

@MSGhero MSGhero commented Oct 20, 2015

The console itself works fine now, but now I need some confirmation that these changes are ok. There are some breaking changes now.

Compiles and works, but I cut/commented out a lot of stuff.
Need to test legacy vs next and flash vs cpp.
@larsiusprime
Copy link
Member

neato

Getting there. Most functions re-implemented. Need an adult soon to check my work. Still need to test cpp.
Everything works but maybe not like it used to.
@MSGhero MSGhero changed the title Hscript console (don't merge yet) Hscript console (I need an adult) Oct 23, 2015
@MSGhero
Copy link
Member Author

MSGhero commented Oct 23, 2015

I'm mainly done now, but yeah it's a breaking change. Since hscript deals with objects directly instead of strings, I removed all the parsing code and string parameters from functions. ie: switchState command now straight up takes a new state: switchState(new TestState()).

@MSGhero
Copy link
Member Author

MSGhero commented Oct 23, 2015

Pausing and TAB don't work on neko, but they probably never did. Didn't test html5 or -Dnext.

@MSGhero
Copy link
Member Author

MSGhero commented Oct 27, 2015

The travis script and unit tests seem ok on my computer, and the build does install hscript. Why can't it find hscript.Expr?

Had to add hscript, hopefully it works now
@Gama11
Copy link
Member

Gama11 commented Oct 27, 2015

As is, every flixel project that doesn't include hscript in the xml manually won't compile - would be better to have that in the include.xml.

In theory it would be nice to only include it if it's installed (and only have the console be usable in that case), but I'm not sure if there's a way to do that with the project xml format...

@MSGhero
Copy link
Member Author

MSGhero commented Oct 27, 2015

I added it to include.xml, but would it go somewhere besides the setup block? Should I wrap all console code in #if hscript?

@Gama11
Copy link
Member

Gama11 commented Oct 27, 2015

setup is only used for lime setup flixel. If you actually want to include the haxelib during compilation, you need the regular <haxelib name="hscript" /> like the include.xml does with openfl.

@MSGhero
Copy link
Member Author

MSGhero commented Oct 27, 2015

Ugh ok. I see what you mean now about always including it. I guess it would always be included for release builds too even when the debugger flxdef is off.

@MSGhero
Copy link
Member Author

MSGhero commented Oct 27, 2015

The unit tests look fine, but it's still breaking coverage on the import hscript.Expr

@Gama11
Copy link
Member

Gama11 commented Oct 27, 2015

The unit tests use a .hxml file for flash.

https://github.com/HaxeFlixel/flixel/blob/dev/tests/unit/test.hxml

@MSGhero
Copy link
Member Author

MSGhero commented Oct 27, 2015

I'm never adding an external lib to flixel again.

@MSGhero
Copy link
Member Author

MSGhero commented Oct 27, 2015

Oh hey, dev cpp is working again. Haxe recently fixed the multiple inheritance issue which was preventing flixel-ui from compiling.

@MSGhero MSGhero changed the title Hscript console (I need an adult) Hscript console Oct 28, 2015
@gamedevsam
Copy link
Contributor

This change looks great! Has anyone been able to test this functionality?

@MSGhero
Copy link
Member Author

MSGhero commented Nov 8, 2015

Me lol. Do you need a rundown of the changes or are you making sure it works for other people?

@gamedevsam
Copy link
Contributor

I was mostly wondering if there was someone in the community that opposes this change or has something to say before I merge it in. @HaxeFlixel/owners any opposition or can I merge this?

@Beeblerox
Copy link
Member

i haven't checked this pull request, but i don't mind replacing current functionality with hscript as far as it works

typedef PathToVariable = {
object:Dynamic,
variableName:String
private class Interp extends hscript.Interp
Copy link
Member Author

Choose a reason for hiding this comment

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

FlxInterp? lol

@MSGhero
Copy link
Member Author

MSGhero commented Nov 8, 2015

I'm not 100% sure if the Watch tab still works all the time, but the watch console command does work.

I'm also not sure why the Event.RENDER bit was in there, maybe a workaround for an old openfl issue?

@Gama11
Copy link
Member

Gama11 commented Nov 27, 2015

The ability to run arbitrary Haxe code from the console is pretty amazing. :)

Also:

202 additions and 809 deletions

👍

Not too mention, seems like it's not as crashy on neko anymore.

@Gama11 Gama11 changed the title Hscript console Hscript console, closes #1156 Nov 27, 2015
Gama11 added a commit that referenced this pull request Nov 27, 2015
@Gama11 Gama11 merged commit b611216 into HaxeFlixel:dev Nov 27, 2015
@MSGhero MSGhero deleted the hscript-console branch November 27, 2015 21:57
@MSGhero
Copy link
Member Author

MSGhero commented Nov 28, 2015

In the original issue, you said that it'd be cool if we could #if hscript to display an error message if not. Is that possible in retrospect, or is hscript a dependency for this lib now?

@Gama11
Copy link
Member

Gama11 commented Nov 28, 2015

It's not possible unless we remove the <haxelib name="hscript" /> from the include.xml, so the user would have to add that manually to his Project.xml (which I'm not a fan of), since there's no way to tell openfl to "include this lib if it's there". If a haxelib isn't there, it errors.

@Gama11
Copy link
Member

Gama11 commented Nov 28, 2015

We could try using Compiler.addClassPath() and check the output of haxelib path hscript in the initialization macro. Unfortunately there's no Compiler.addHaxelib() (HaxeFoundation/haxe#4678).
Not sure if that would confuse OpenFL in some way.

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.

5 participants