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
ACTIONSCRIPT: Initial ActionScript byte code interpreter #294
Conversation
844f6bb
to
f565d3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat!
There's some issues I'd like you to address first, though. See below. Also, AppVeyor fails for this PR. I think you forgot to include somewhere.
|
||
} // End of namespace ActionScript | ||
|
||
} // End of namespace Aurora |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing end-of-line at the end of the file
src/aurora/actionscript/asbuffer.cpp
Outdated
size_t startPos = _script->pos(); | ||
|
||
switch (opcode) { | ||
case kActionStop: actionStop(avm);break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use spaces for alignment. As you can see, when you mix spaces and tabs for alignment, it suddenly doesn't align.
(Of course, the indentation at the start of the line should still be tabs)
src/aurora/actionscript/asbuffer.cpp
Outdated
void ASBuffer::execute(AVM &avm) { | ||
byte opcode; | ||
if (_printAssembly) | ||
info("-----"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should rather be a debug channel.
See, for example, how src/aurora/nwscript/ncsfile.cpp
handles it with debugC()
. You should add a relevant debug channel into the Common::DebugManager
.
(Possible, the "kDebugScripts" channel should be renamed to say that it's about NWScript. Lua (and now ActionScript) should be their own debug channels)
src/aurora/actionscript/variable.cpp
Outdated
#include "src/aurora/actionscript/variable.h" | ||
#include "src/aurora/actionscript/object.h" | ||
#include "variable.h" | ||
#include "function.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two need a full path
src/aurora/actionscript/function.h
Outdated
#include <vector> | ||
|
||
#include "src/aurora/actionscript/variable.h" | ||
#include "asbuffer.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Full paths, please
src/aurora/actionscript/avm.h
Outdated
* @param value | ||
* The parameters of the fscommand | ||
*/ | ||
void fscommand(const Common::UString &name, const Common::UString &value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fsCommand
, then, I guess?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes and no, since the fscommand is written this way in the actionscript code, i wanted to make an exception here, to match it with the actionscript specification. But if you want i will change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please change it, thanks
src/aurora/actionscript/asbuffer.cpp
Outdated
|
||
#include <boost/none_t.hpp> | ||
#include <src/common/bitstream.h> | ||
#include <src/common/memreadstream.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include our own includes with #include ""
, not #include <>
f565d3d
to
65c5eda
Compare
65c5eda
to
31dc0ba
Compare
The issues should now be fixed and appveyor works again. |
src/aurora/actionscript/asbuffer.h
Outdated
|
||
class ASBuffer { | ||
public: | ||
ASBuffer(Common::SeekableReadStream *as, bool printAssembly = false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter printAssembly
can go now
src/aurora/actionscript/asbuffer.cpp
Outdated
|
||
void ASBuffer::execute(AVM &avm) { | ||
byte opcode; | ||
debugC(kDebugEngineActionScript, 2, "--- Start Actionscript ---"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Start is debug level 2, end is debug level 1? Is that correct?
src/common/debugman.h
Outdated
kDebugEngineLogic , ///< "ELogic", engine game logic. | ||
|
||
kDebugEngineScripts , ///< "EScripts", engine nwscripts | ||
kDebugEngineActionScript, ///< "EActionScript", engine actionscript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kDebugEngineActionScript is the wrong debug channel for the action script there.
If you look into NWScript, it uses kDebugScripts for the general script disassembly, things that are the same for every engine. kDebugEngineScripts is for things that the engine modifies or does, like engine functions.
I.e. you should rather use kDebugActionScript for most (all?) of the debug stuff in the ActionScript interpreter there.
31dc0ba
to
ad3511b
Compare
The issues should now be fixed |
src/aurora/actionscript/asbuffer.cpp
Outdated
|
||
namespace ActionScript { | ||
|
||
const static byte kActionNextFrame = 0x04; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should probably rather be an enum.
src/aurora/actionscript/asbuffer.cpp
Outdated
size_t startPos = _script->pos(); | ||
|
||
switch (opcode) { | ||
case kActionStop: actionStop(avm);break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a space before the "break;"? And also an extra one after the ":", so that "kActionDefineFunction2:" isn't squished against the actionDefineFunction2() without any space.
ad3511b
to
d0aacf6
Compare
The issues should now be fixed |
Merged as 3efa87d...bb83638, thanks! :) |
This is an initial attempt for an actionscript interpreter, usable for the dragonage and dragonage2 scaleform ui. It is not feature complete, to interpret all actionscript used in both dragon age games, mainly because scaleform utilizes a subset of the flash Api which needs to be implemented as additional PRs. This should be considered a Proof of Concept demo.