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

10220 - Control Variables New Entry: GetTime #3124

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

Conversation

jetrotal
Copy link
Contributor

@jetrotal jetrotal commented Oct 15, 2023

This one was requested a lot by the Creative Unconscious devs,
They want to include special events related to christmas date or other holydays:

10220 - Control Variables New Entry: GetTime

Gets current date information and attach it to a variable.
image

Usage:

@raw 10220, "Type_of_Date", targetVar_IsVar, targerVar
// "set year to variable 1:"
@raw 10220, "getYear", 0, 1
// "set month to variable 2:"
@raw 10220, "getMonth", 0, 2
// "set day to variable 3:"
@raw 10220, "getDay", 0, 3
// "set hour to variable 4:"
@raw 10220, "getHour", 0, 4
// "set minute to variable 5:"
@raw 10220, "getMinute", 0, 5
// "set second to variable 6:"
@raw 10220, "getSecond", 0, 6
// "set day of week to variable 7:"
@raw 10220, "getWeekDay", 0, 7
// "set days in year to variable 8:"
@raw 10220, "getYearDay", 0, 8
// "set daylight savings flag to variable 9:"
@raw 10220, "getdaylightsavings", 0, 9
// "set the entire timestamp to variable 10:"
@raw 10220, "gettimestamp", 0, 10

@Mimigris
Copy link

Hmmm, I didn't tested the pull request, though I think that having the possibility to directly have a way to get these info is a really good idea. Though, while I know that in terms of writing the format of a command, you may be limited by how the Maniac patch allows you to write things, but wouldn't it make more sense to extend the Control Variables command instead of creating a new command for that? I think it would be a weird thing to create a command just for that instead of extending the Control Variables command, which would allow a bit more possibilities, as well as not creating thousands of commands each time a new command to get a value is created.

@jetrotal
Copy link
Contributor Author

jetrotal commented Oct 15, 2023

wouldn't it make more sense to extend the Control Variables command instead of creating a new command for that?

It would be harder to create and maintain commands, since neither vanilla nor maniacs have a way to allow this type of change on a pre existing command.

The user would have to edit a xml version of his map or ldb, then editing the command on any way would break it.

I could rename this command to xtraVariable to attach more other interesting data to it.

@carstene1ns
Copy link
Member

since neither vanilla nor maniacs have a way to allow this type of change on a pre existing command.

Just use EasyRPG Editor for this? 😅

Also, should be possible to patch this in. Cherry has added a lot of functionality through binary patches in the official editor. WeSomeone could add a compatibility shim to allow such things.

@jetrotal
Copy link
Contributor Author

Just use EasyRPG Editor for this? 😅

Having an unfinished editor just for command snippets sounds worse than using @raw commands from a vastly used patch.

The editor is not fully usable rn, we would have to work on it for 2 or 3 years to have it fully working, and let the CU community use it.

Also, should be possible to patch this in. Cherry has added a lot of functionality through binary patches in the official editor. We Someone could add a compatibility shim to allow such things.

Again, we already have have maniacs patch @raw commands letting us access and write new functions. Since the patch It's widelly used, it makes more sense to use it instead creating yet another patch.

IMHO, We should bring users of different patches closer to easyRPG, instead of creating more and more alternatives to tweak the editor.


The common response for asking for new features was "no, wait for the editor".
Those raw commands is finally a way to do it ourselves and let those new features be future proof, since all new commands are using the same executeCommand(com) functions that every other command vanilla command uses.

src/game_interpreter.cpp Outdated Show resolved Hide resolved
Gets current date information and attach it to a variable.

Usage:
```js
@raw 2054, "Type_of_Date", targetVar_IsVar, targerVat

```
```js
// "set year to variable 1:"
@raw 2054, "getYear", 0, 1
// "set month to variable 2:"
@raw 2054, "getMonth", 0, 2
// "set day to variable 3:"
@raw 2054, "getDay", 0, 3
// "set hour to variable 4:"
@raw 2054, "getHour", 0, 4
// "set minute to variable 5:"
@raw 2054, "getMinute", 0, 5
// "set second to variable 6:"
@raw 2054, "getSecond", 0, 6
// "set day of week to variable 7:"
@raw 2054, "getWeekDay", 0, 7
// "set days in year to variable 8:"
@raw 2054, "getYearDay", 0, 8

```
@Ghabry Ghabry added the Needs feedback Waiting for the issue author to give further information. label Oct 26, 2023
@Ghabry
Copy link
Member

Ghabry commented Oct 26, 2023

There are valid concerns raised if this should be part of the "Control Variables" command. I will check first if there is a non-shitty way to achieve this currently...

With lcf2xml for sure but most are using this closed source TPC thing.

@jetrotal
Copy link
Contributor Author

There are valid concerns raised if this should be part of the "Control Variables" command. I will check first if there is a non-shitty way to achieve this currently...

With lcf2xml for sure but most are using this closed source TPC thing.

All I can think is whitelisting some of DynRPG commands to work even without it enabled.
Then create non destructive ways to run @raw commands from those comments, similar to #3110.

@Ghabry
Copy link
Member

Ghabry commented Oct 30, 2023

With that raw workaround proposed this can move into ControlVariables.

Suggested boilerplate:

diff --git a/src/game_interpreter.cpp b/src/game_interpreter.cpp
index c0c1aa9ec..02c6af39d 100644
--- a/src/game_interpreter.cpp
+++ b/src/game_interpreter.cpp
@@ -1250,6 +1250,26 @@ bool Game_Interpreter::CommandControlVariables(lcf::rpg::EventCommand const& com
 			// Expression (Maniac)
 			value = ManiacPatch::ParseExpression(MakeSpan(com.parameters).subspan(6, com.parameters[5]), *this);
 			break;
+		case 200: { // EasyRPG Command
+			auto cmd_size = com.parameters.size();
+			int op, subop;
+			int arg1 = 0;
+			int arg2 = 0;
+			if (cmd_size >= 7) {
+				op = com.parameters[5];
+				subop = com.parameters[6];
+				if (cmd_size >= 8) {
+					arg1 = com.parameters[7];
+				}
+				if (cmd_size >= 9) {
+					arg2 = com.parameters[8];
+				}
+				value = ControlVariables::EasyRpgCommand(op, subop, arg1, arg2);
+			} else {
+				value = 0;
+			}
+			break;
+		}
 		default:
 			Output::Warning("ControlVariables: Unsupported operand {}", operand);
 			return true;
diff --git a/src/game_interpreter_control_variables.cpp b/src/game_interpreter_control_variables.cpp
index d8c0590dd..903970038 100644
--- a/src/game_interpreter_control_variables.cpp
+++ b/src/game_interpreter_control_variables.cpp
@@ -488,3 +488,24 @@ int ControlVariables::Divmul(int arg1, int arg2, int arg3) {
 int ControlVariables::Between(int arg1, int arg2, int arg3) {
 	return (arg1 >= arg2 && arg2 <= arg3) ? 0 : 1;
 }
+
+int ControlVariables::EasyRpgCommand(int op, int subop, int arg1, int arg2) {
+	(void)arg1;
+	(void)arg2; // Remove when args are used
+
+	enum class Op {
+		GetDate = 0
+	};
+
+	switch (static_cast<Op>(op)) {
+		case Op::GetDate:
+			switch (subop) {
+				case 0: // TODO
+					break;
+			};
+			return 0;
+			break;
+		default:
+			return 0;
+	}
+}
diff --git a/src/game_interpreter_control_variables.h b/src/game_interpreter_control_variables.h
index a534b83b7..1fcbc717f 100644
--- a/src/game_interpreter_control_variables.h
+++ b/src/game_interpreter_control_variables.h
@@ -45,6 +45,7 @@ namespace ControlVariables {
 	int Muldiv(int arg1, int arg2, int arg3);
 	int Divmul(int arg1, int arg2, int arg3);
 	int Between(int arg1, int arg2, int arg3);
+	int EasyRpgCommand(int op, int subop, int arg1, int arg2);
 }
 
 #endif

@jetrotal jetrotal changed the title New Command: 2054 - GetTime 10220 - Control Variables New Entry: GetTime Nov 8, 2023
@jetrotal
Copy link
Contributor Author

jetrotal commented Nov 8, 2023

Implemented changes you guys asked in a straight forward way, since controlVariables doesn't care about strings at all...

It works fine with Maniacs @raw command, but It's impossible to edit and looks ugly AF:
image

that will be solved once we implement #3137, The code will look like:
@easyrpg_raw @ControlVariables, "getDay", 0, 1

@Ghabry Ghabry added the EasyRPG New functionality exclusive to EasyRPG Player label Apr 20, 2024
@fdelapena fdelapena added the Awaiting Rebase Pull requests with conflicting files due to former merge label Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Rebase Pull requests with conflicting files due to former merge EasyRPG New functionality exclusive to EasyRPG Player Enhancement Needs feedback Waiting for the issue author to give further information.
Development

Successfully merging this pull request may close these issues.

5 participants