NicoM1 / IceEntity

A simple framework for managing entitys, components, and live-at-runtime scripts in haxeflixel
MIT License
58 stars 7 forks source link

Auto-complete compliant scripting #11

Closed Ohmnivore closed 10 years ago

Ohmnivore commented 10 years ago

Now IceEntity can parse these sort of files:

package;

import flixel.FlxG; //request Player;

class TestScript { public function new() { FlxG.log.add("TestFunction"); }

public function reload():Void
{

}

public function update():Void
{

}

public function destroy():Void
{

}

}

Ohmnivore commented 10 years ago

ugh, that came out horrible, a gist is better : https://gist.github.com/Ohmnivore/688c897b9372256b739a#file-script-hx

Ohmnivore commented 10 years ago

init was replaced by new, and the only special tag now is "//request".

In the end the functions are directly parsed, no tags necessary.

NicoM1 commented 10 years ago

ok, looking through the code right now, takes me a while to understand code I didn't write, one question: any reason we cant just use "request"? why do we need comments? (does it screw up auto-complete?, probably...)

NicoM1 commented 10 years ago

also, as this is the dev version, the flxtypedgroup import is not needed, as IceEntity's dev corresponds to flixels dev (I recommend you switch)

NicoM1 commented 10 years ago

Really sorry about all the notes, if its too much for you to change, I can work on it and then let you checkover what I do:D main points: really like what you've done, do want to tweak to allow messier code, and also: I do worry a bit about the performance hit of iterating over every character at every reload, but I guess thats one of those premature optimization worries, and we should only care if it's actually noticeable (plus, actually, currently the script is only re-parsed if the text changes, so a little blip after a tweak shouldn't be a big deal)

Ohmnivore commented 10 years ago

No need to be sorry, you raised some valid points. The reason why I changed init to new was because I think that auto-complete complains if a class doesn't have a constructor. Instead of an empty constructor, I think it's better if init was morphed into new. For the "public function" part, I added public because it makes sense since we're pretending the script is a class. Much like in Unity, the script's update/destroy/init functions are expected to be called by other classes. Also, since constructors are public, the other functions are visually aligned if they're also declared public.

For the spacing issues, I admit I was rather strict. Because you have a clearer idea than I do of how to fix that, and the fact that you have done that with the last implementation, I'll leave it up to you.

About if (l.substr(0, 6) == "import"), it should work regardless of leading whitespace because l is trimmed before the if statement.

I'm not sure about this, but I think regex are a few times more expensive than other string operations. Also I'm not that worried about iterating over every character in the function parsing function (pun pun pun), since that's probably what happens during an indexOf call anyways.

NicoM1 commented 10 years ago

I'm going to attempt regex's anyway, but only for very small strings... working on my changes as we speak, I think parsing should be done so it ignores the "public" that way if you prefer the look it will compile, but there is no need to actually check for it...

NicoM1 commented 10 years ago

ok pretty much all done, but reload appears to not be being called, gonna look into it

NicoM1 commented 10 years ago

hmmm nothing is reloading actually....

NicoM1 commented 10 years ago

so for some reason, on reload, when it calls: updateScript = ScriptHandler.Parse("update", script); the result is null, while when it calls the exact same function at startup it works

NicoM1 commented 10 years ago

@Ohmnivore ooohhh this ended up being a nasty bug, I believe my changes are working now, about to submit a pull so we can discuss it:D

NicoM1 commented 10 years ago

closing, discussion can be moved to #13