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

Maniac Patch - CallCommand Update #3319

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jetrotal
Copy link
Contributor

Partial Change to cover the new variations of Maniac's "call command"

Those changes make it way easier when writing and maintaining new commands.
image

This could be merged, though it lacks the support of expressions.

Looks like Typical expressions only affects single outputs.
But this seems to manipulate an "array of expressions", which I failed to figure out how could this work.

Changes to cover the new variations of Maniac's "call command"
@jetrotal jetrotal changed the title Maniac Patch - CallCommand Update - init Maniac Patch - CallCommand Update Dec 30, 2024
@jetrotal jetrotal force-pushed the CallCommand-Updates branch from 11b96e7 to bbcb8ff Compare December 30, 2024 13:11
@Ghabry Ghabry added this to the 0.8.2 milestone Jan 7, 2025
@Ghabry
Copy link
Member

Ghabry commented Jan 9, 2025

This array syntax discussion is misleading. It renders it as an array but actually it is just multiple expressions chained after each other. So this is actually solvable without implementing array support (woohoo).

Tech Demo:

grafik

@Ghabry
Copy link
Member

Ghabry commented Jan 9, 2025

Add below ParseExpression in Maniac_patch.cpp:

std::vector<int32_t> ManiacPatch::ParseExpressions(Span<const int32_t> op_codes, const Game_BaseInterpreterContext& interpreter) {
	std::vector<int32_t> ops;
	for (auto& o : op_codes) {
		auto uo = static_cast<uint32_t>(o);
		ops.push_back(static_cast<int32_t>(uo & 0x000000FF));
		ops.push_back(static_cast<int32_t>((uo & 0x0000FF00) >> 8));
		ops.push_back(static_cast<int32_t>((uo & 0x00FF0000) >> 16));
		ops.push_back(static_cast<int32_t>((uo & 0xFF000000) >> 24));
	}

	if (ops.empty()) {
		return {};
	}

	auto it = ops.begin();

	std::vector<int32_t> results;

	while (true) {
		results.push_back(Process(it, ops.end(), interpreter));

		if (it != ops.end() && static_cast<Op>(*it) == Op::Null) {
			break;
		}
	}

	return results;
}

Header:

#ifndef EP_MANIAC_PATCH
#define EP_MANIAC_PATCH

#include <array>
#include <cstdint>
#include <vector> <- NEW
#include "span.h"

#include "game_strings.h"

class Game_BaseInterpreterContext;

namespace ManiacPatch {
	int32_t ParseExpression(Span<const int32_t> op_codes, const Game_BaseInterpreterContext& interpreter);
	std::vector<int32_t> ParseExpressions(Span<const int32_t> op_codes, const Game_BaseInterpreterContext& interpreter); <- NEW (pls remove the "<- NEW" markers before commit)

In CallCommand:

	case 4: { // Expression mode - interprets its own parameters as Maniac Expressions

		//TODO - LEARN HOW THIS ACTUALLY WORKS
		//break;

		auto values = ManiacPatch::ParseExpressions(MakeSpan(com.parameters).subspan(5), *this);
                // TODO: Parsed numbers are in "values", forward them to the command

Guess you can figure the rest out yourself :)

@Ghabry Ghabry modified the milestones: 0.8.2, 0.8.1 Jan 9, 2025
@jetrotal
Copy link
Contributor Author

jetrotal commented Jan 9, 2025

Almost fully working!

Actor id from actor[2].atk is being idenfied as 0 instead of 2.

@cmd 1, "", `[1, 2 + 3, 4 * v[5], actor[2].atk, rnd(7, 8)]

image

@Ghabry
Copy link
Member

Ghabry commented Jan 10, 2025

Thats indeed a bug: In Fn::Actor I forgot to add the imm3 assignment (you can see in the other cases).

Oh and the abort-condition in this while loop is wrong.

It must be

if (it == ops.end() || static_cast<Op>(*it) == Op::Null)

@jetrotal
Copy link
Contributor Author

@Ghabry should those other also contain imm3?

	case Fn::Misc:
		if (imm2 != 1) {
			Output::Warning("Maniac: Expression misc args {} != 1", imm2);
			return 0;
		}
		return ControlVariables::Other(Process(it, end, ip));
	case Fn::Pow:
		if (imm2 != 2) {
			Output::Warning("Maniac: Expression pow args {} != 2", imm2);
			return 0;
		}
		return ControlVariables::Pow(Process(it, end, ip), Process(it, end, ip));
	case Fn::Sqrt:
		if (imm2 != 2) {
			Output::Warning("Maniac: Expression sqrt args {} != 2", imm2);
			return 0;
		}
		return ControlVariables::Sqrt(Process(it, end, ip), Process(it, end, ip));
	case Fn::Sin:
		if (imm2 != 3) {
			Output::Warning("Maniac: Expression sin args {} != 3", imm2);
			return 0;
		}
		return ControlVariables::Sin(Process(it, end, ip), Process(it, end, ip), Process(it, end, ip));
	case Fn::Cos:
		if (imm2 != 3) {
			Output::Warning("Maniac: Expression cos args {} != 3", imm2);
			return 0;
		}
		return ControlVariables::Cos(Process(it, end, ip), Process(it, end, ip), Process(it, end, ip));
	case Fn::Atan2:
		if (imm2 != 3) {
			Output::Warning("Maniac: Expression atan2 args {} != 3", imm2);
			return 0;
		}
		return ControlVariables::Atan2(Process(it, end, ip), Process(it, end, ip), Process(it, end, ip));
	case Fn::Min:
		if (imm2 != 2) {
			Output::Warning("Maniac: Expression min args {} != 2", imm2);
			return 0;
		}
		return ControlVariables::Min(Process(it, end, ip), Process(it, end, ip));
	case Fn::Max:
		if (imm2 != 2) {
			Output::Warning("Maniac: Expression max args {} != 2", imm2);
			return 0;
		}
		return ControlVariables::Max(Process(it, end, ip), Process(it, end, ip));
	case Fn::Abs:
		if (imm2 != 1) {
			Output::Warning("Maniac: Expression abs args {} != 1", imm2);
			return 0;
		}
		return ControlVariables::Abs(Process(it, end, ip));
	case Fn::Clamp:
		if (imm2 != 3) {
			Output::Warning("Maniac: Expression clamp args {} != 3", imm2);
			return 0;
		}
		return ControlVariables::Clamp(Process(it, end, ip), Process(it, end, ip), Process(it, end, ip));
	case Fn::Muldiv:
		if (imm2 != 3) {
			Output::Warning("Maniac: Expression muldiv args {} != 3", imm2);
			return 0;
		}
		return ControlVariables::Muldiv(Process(it, end, ip), Process(it, end, ip), Process(it, end, ip));
	case Fn::Divmul:
		if (imm2 != 3) {
			Output::Warning("Maniac: Expression divmul args {} != 3", imm2);
			return 0;
		}
		return ControlVariables::Divmul(Process(it, end, ip), Process(it, end, ip), Process(it, end, ip));
	case Fn::Between:
		if (imm2 != 3) {
			Output::Warning("Maniac: Expression between args {} != 3", imm2);
			return 0;
		}
		return ControlVariables::Between(Process(it, end, ip), Process(it, end, ip), Process(it, end, ip));
	default:
		Output::Warning("Maniac: Expression Unknown Func {}", imm);
		for (int i = 0; i < imm2; ++i) {
			Process(it, end, ip);
		}
		return 0;
}

other than that, seems fine

@Ghabry
Copy link
Member

Ghabry commented Jan 10, 2025

nope, they should be correct. none of them uses a third parameter (imm3)

output_args.reserve(arr_length);
for (int i = 0; i < arr_length; ++i) {
output_args.push_back(Main_Data::game_variables->Get(arr_begin + i));
int processing_mode = (com.parameters[0] >> 4) & 0b1111;
Copy link
Member

Choose a reason for hiding this comment

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

I thought now for 10 minutes about this line:

For processing_mode when looking at the bits, assuming its a 16 bit number you grab the A:

param[0] -> 0000 0000 AAAA 0000

And in case0/1 you do int arr_begin = ValueOrVariableBitfield(com.parameters[0], 1, com.parameters[2]);

First two args are the mode and the shift.

This is equal to

param[0] -> 0000 0000 AAAA 0000

So its the same as the processing_mode.

...

Thinking more about this that means that for the cases 0, 1, 2 it is "direct/variable/indirect" (you don't handle case 2 yet)

To make this less confusing for me change

int arr_begin = ValueOrVariableBitfield(com.parameters[0], 1, com.parameters[2]);

to

int arr_begin = ValueOrVariable(processing_mode, com.parameters[2]);

This should give the same result (please test xD)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is legacy code, I kept exactly as is to skip it, while implementing the newer modes.

Have to take a look on how to test it.

break;
}

case 3: { // Dynamic mode - read from its own parameters
Copy link
Member

Choose a reason for hiding this comment

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

Can you please explain in words how the DynamicMode is encoded?

I trust you that it works but I really do not understand why (and I want to rewrite it to make it more clear).

Especially this weird bit-shifting o_O

Copy link
Contributor Author

@jetrotal jetrotal Jan 10, 2025

Choose a reason for hiding this comment

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

Dynamic Code Examples:

@cmd 50, "", [91, 92, 93];
 {
  "code": 3019,
  "indent": 0,
  "string": {},
  "parameters": " 48 50 8 3 0 91 92 93 "
}

first arg (48) reffers the mode (default/dynamic/expressions)
image
It can also store the type of other areas, I can't quite remember....

second arg (52) is the command ID

third (8) is ??? - Can't remember, it was a type of offset...

fourth (3) is the length of the array.

fifth (0) I guess it was a separator?

sixth and onwards (until the end of its length) seems to be the array itself.

sometimes (cases bellow), some extra values (starting after the length is reached), represents if each array entry type (value, var, indirect var)

@cmd 51, "", [v[101], v[102], 3, v[104]] ;  
 {
  "code": 3019,
  "indent": 0,
  "string": {},
  "parameters": " 48 51 9 4 0 101 102 3 104 4113 "
}

@cmd 52, "", [v[v[101]], v[v[102]], v[v[103]], 4, v[5]] ;   
{
  "code": 3019,
  "indent": 0,
  "string": {},
  "parameters": " 48 52 10 5 0 101 102 103 4 5 546 1 "
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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


case 4: { // Expression mode - interprets its own parameters as Maniac Expressions

auto values = ManiacPatch::ParseExpressions(MakeSpan(com.parameters).subspan(5), *this);
Copy link
Member

Choose a reason for hiding this comment

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

output_args = ManiacPatch... should work here. They are the same type.

@jetrotal jetrotal force-pushed the CallCommand-Updates branch 2 times, most recently from 30a7d58 to 6c07529 Compare January 10, 2025 15:08
@Ghabry Ghabry force-pushed the CallCommand-Updates branch from 5209bc8 to 6fee86c Compare January 10, 2025 16:24
@Ghabry
Copy link
Member

Ghabry commented Jan 10, 2025

Think I finally got mode 3 right.

Test data:

@cmd v[v[1234]], "", [100, v[101], v[v[102]], 200, v[201], v[v[202]], 300, v[301], v[v[302]], 400, v[401], v[v[402]], 500, 501, 502, 600, 601, 602, 700, 701, 702, 800, 801, 802, 900, 901, 902, 1000, 1001, 1002, 2000, 2001, 2002, 3000, 3001, 3002, 4000, 4001, 4002, 4003, 4004, 5000, 5001, 5002, 5003, 5004]
100,
v[101],
v[v[102]],
200,
v[201],
v[v[202]],
300,
v[301],
v[v[302]],
400,
v[401],
v[v[402]],
500,
v[501],
v[v[502]],
600,
v[601],
v[v[602]],
700,
v[701],
v[v[702]],
800,
v[801],
v[v[802]],
900,
v[901],
v[v[902]],
1000,
v[1001],
v[v[1002]],
2000,
v[2001],
v[v[2002]],
3000,
v[3001],
v[v[3002]],
4000,
4001,
4002,
4003,
4004,
5000,
5001,
5002,
5003,
5004

Dumped output directly from VariableOrVariableBitfield: (m is the mode, 0 direct, 1 v[], 2 v[v[]])

oob means "out of bounds" (when all remaining parameters are "direct" the mode is omitted)

Debug: m=0, s=0, v=100
Debug: m=1, s=1, v=101
Debug: m=2, s=2, v=102
Debug: m=0, s=3, v=200
Debug: m=1, s=0, v=201
Debug: m=2, s=1, v=202
Debug: m=0, s=2, v=300
Debug: m=1, s=3, v=301
Debug: m=2, s=0, v=302
Debug: m=0, s=1, v=400
Debug: m=1, s=2, v=401
Debug: m=2, s=3, v=402
Debug: m=0, s=0, v=500
Debug: m=1, s=1, v=501
Debug: m=2, s=2, v=502
Debug: m=0, s=3, v=600
Debug: m=1, s=0, v=601
Debug: m=2, s=1, v=602
Debug: m=0, s=2, v=700
Debug: m=1, s=3, v=701
Debug: m=2, s=0, v=702
Debug: m=0, s=1, v=800
Debug: m=1, s=2, v=801
Debug: m=2, s=3, v=802
Debug: m=0, s=0, v=900
Debug: m=1, s=1, v=901
Debug: m=2, s=2, v=902
Debug: m=0, s=3, v=1000
Debug: m=1, s=0, v=1001
Debug: m=2, s=1, v=1002
Debug: m=0, s=2, v=2000
Debug: m=1, s=3, v=2001
Debug: m=2, s=0, v=2002
Debug: m=0, s=1, v=3000
Debug: m=1, s=2, v=3001
Debug: m=2, s=3, v=3002
Debug: m=0, s=0, v=4000
Debug: m=0, s=1, v=4001
Debug: m=0, s=2, v=4002
Debug: m=0, s=3, v=4003
Debug: m=0, s=0, v=4004
Debug: m=0, s=1, v=5000
Debug: m=0, s=2, v=5001
Debug: m=0, s=3, v=5002
Debug: oob v=5003
Debug: oob v=5004

@jetrotal
Copy link
Contributor Author

that works for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants