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

Improve AptEditor & Apt VM #593

Draft
wants to merge 106 commits into
base: master
Choose a base branch
from

Conversation

BillStark001
Copy link

I implemented a prototype of the EA-modified ActionScript VM. The main TO-DO list includes AS DOM, built-in function implementations, script compiler, and Apt file writer.

@BillStark001 BillStark001 marked this pull request as draft July 14, 2021 10:10
This reverts commit cabee61.
Copy link
Contributor

@feliwir feliwir left a comment

Choose a reason for hiding this comment

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

Hey, thanks a lot for your contribution 👍 This looks great already!
I know this is still WIP, but some early comments already:

  1. On which games are you testing this? We must make sure the Apt VM stays compatible with earlier games ( BFME in particular ). We mostly tested against those games.
  2. For your final PR make sure we don't have too much commented out code

src/OpenSage.Game/Gui/Apt/ActionScript/Value.cs Outdated Show resolved Hide resolved
src/OpenSage.Game/Data/Apt/AptFile.cs Outdated Show resolved Hide resolved
@@ -81,10 +81,15 @@ void OnWindowResized()
ImGui.TextWrapped("Start OpenSage Apt Editor by providing a root path.");
ImGui.TextWrapped("It's recommended that you set it to " +
"the RA3SDK_UI_ScreensPack folder so you could load apt files more easily.");

// TODO More fancy implementations?
var defaultInputPath = "G:\\Games\\RA#s\\aptuis\\aptui";
Copy link
Contributor

Choose a reason for hiding this comment

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

This can't stay this way

@feliwir feliwir changed the title Origin/apt Improve AptEditor & Apt VM Jul 14, 2021
@BillStark001
Copy link
Author

Sorry for creating a pull request with so many unfinished codes. The underlying logic is still incomplete to a large extent, I plan to clean up the code after I finished mostly the resource-loading logic, built-in AS functions, and opcodes.
This PR is only a draft as proposed.

@feliwir
Copy link
Contributor

feliwir commented Jul 14, 2021

@BillStark001 no problem, i saw this is a draft :) Just wanted to comment early, so you know we're looking at it. Also feel free to join our Discord to discuss this stuff 👍

instruction = new GotoLabel();
parameters.Add(Value.FromString(reader.ReadStringAtOffset()));
break;
case InstructionType.PushData: // NIE doubtful and not used in any games
Copy link
Contributor

@feliwir feliwir Jul 16, 2021

Choose a reason for hiding this comment

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

This instruction is used by the Battle for Middleearth games, which were the first SAGE games that used Apt for UI. Please make sure you stay compatible in that regard. Those are 3 games (BFME I, BFME II and BFME II Rise of the Witchking).

Copy link
Author

Choose a reason for hiding this comment

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

I see. I just added a line of throwing an exception to remind me where it is used. Although I'm mainly developing with RA3's scripts, I'll check the compatibility of BFME.

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.

None yet

4 participants