Technical Improvements #1

[Equinoxtic]

Hallo, I really like and love this engine, I thought I would try contributing to it as a gesture <3 (Also kind of my first contribution to any open-source project)

• First, I wanted to improve the countdown code, which will allow to set certain parameters whether it can be enabled, plays sounds, animations, and etc. It may open up to more customizability when it comes to songs, or just some additional options

// PlayState Code Change Example

var countdown:Countdown = new Countdown({
	event: f(EVENT_NAME, EventManager.get(CountdownEvent).recycle(
		index,
		1,
		sounds[index],
		sprites[index],
		0.6, true, null, null, null)),
	enabled: true,
	playSound: true,
	animationPreset: DEFAULT,
	duration: (Conductor.crochet / 1000),
	speed: 1.0
});

• Second, I wanted to compress and lessen some lines on the backend side of the code. This includes iterating over an array of strings instead of repeatedly using Sys.println(...), and making good use of conditional compilation flags to get the executables for the AsyncUpdater class

// Instead of this
Sys.println("-- Help --");
Sys.println("-help                | Show this help");
Sys.println("-nocolor             | Disables colors in the terminal");
Sys.println("-nogpubitmap         | Forces GPU only bitmaps off");
Sys.println("-nocwdfix            | Turns off automatic working directory fix");
// ...and so on

// Do this
final STRINGS:Array<String> = [
	"--- Codename Engine Command Line Help ---",
	"",
	"-help                | Show this help",
	"-nocolor             | Disables colors in the terminal",
	"-nogpubitmap         | Forces GPU only bitmaps off",
	"-nocwdfix            | Turns off automatic working directory fix",
	...
];

for (s in STRINGS) {
	Sys.println(s);
}

// UpdateUtil.hx (2 new functions)
public static function getExecName():String
{
	// Since previously, both linux and mac don't have the .exe extension, we might as well just compress this to a '#else' clause.
	return #if windows "CodenameEngine.exe" #else "CodenameEngine" #end; // weird syntax??? :skull: haxe is just full of surprises
}

public static function getGitExecName():String
{
	// Only add the suffix of the platform we need to get.
	var target:String = #if windows "windows.exe" #end
						#if mac "mac" #end
						#if linux "linux" #end;
	return 'update-${target}';
}

// AsyncUpdater.hx
public static var executableName:String = UpdateUtil.getExecName();
public static var executableGitHubName:String = UpdateUtil.getGitExecName();

[r6915ee]

This pull request can only be tested using the GitHub CLI in the most generic case, at least on my end. I prefer regular Git to handle these operations instead to offer the pure branch name. If I were you, I'd also probably split this into separate pull requests and have these be separate branches to offer the most secure way of handling merging, but that might just be me being picky about my branching as I usually am.

Now, for the review.

[Equinoxtic]

This pull request can only be tested using the GitHub CLI in the most generic case, at least on my end. I prefer regular Git to handle these operations instead to offer the pure branch name. If I were you, I'd also probably split this into separate pull requests and have these be separate branches to offer the most secure way of handling merging, but that might just be me being picky about my branching as I usually am.

Now, for the review.

I'll take that as an advice, it's kind of my first time submitting PRs and whatnot and I only know the basics of Git and stuff T-T

[r6915ee]

This pull request can only be tested using the GitHub CLI in the most generic case, at least on my end. I prefer regular Git to handle these operations instead to offer the pure branch name. If I were you, I'd also probably split this into separate pull requests and have these be separate branches to offer the most secure way of handling merging, but that might just be me being picky about my branching as I usually am.

Now, for the review.

A small update, but the review was partially delayed because I had found an issue with my own custom version management program that prevented Lime from running properly in non-Visual Studio Code environments. I've issued a fix to it on my end, and, at the moment, I'm able to compile properly without issues. I'll review it once it compiles cleanly, since I had removed the export folder for troubleshooting prior.

[r6915ee]

The help message appears to work as intended. However, I do feel that the way the countdown is set up appears to be somewhat odd.

[r6915ee]

I do wish to address this particular hunk. What would be the use case of not being able to manage most of the properties of Countdown in the event itself, like with the other events in PlayState?

[Equinoxtic]

Hm, yeah, now that I think about it, it does feel like it's set up a little oddly. I thought of making an event as a parameter and just handling it (the event) within the Countdown class itself. Now that this has been brought up I start to wonder whether or not it'll break some backend functionality but it seemed to work as intended.

Reverting back to old CNE code where it has an event variable (in PlayState's countdown() method) and using that in the event parameter when making a new Countdown makes no difference (readability wise, I think it's better).

It's an odd setup I know 😅, and my apologies if I replied late. I was asleep by that time. I'll try coming up with an alternative solution later. For now, this is all I've really got.