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

chore: CppScripts Cleanup #1571

Conversation

Tiernan-Alderman
Copy link
Contributor

@Tiernan-Alderman Tiernan-Alderman commented May 13, 2024

Fixes #1511
Desc: Changed the CppScripts file to use a lambda map to load scripts rather than the if else chain. This should make it easier to make future changes as well as reduce code size (especially with an issue with number of if-else statements for some operating machines).

Motivation: Received notice of the issue from a head developer

Type of Changes: Made changes to the CppScripts main file, which now use a lambda. Made a few comment changes but tried to keep all original comments. Overall file lines reduced drastically.

How Has This Been Tested?:
Emosewamc helped write a test file which we used to verify the scripts between the old scripts and new script changes. It returned successful tests when running ctest in the /build directory

Tiernan-Alderman and others added 4 commits May 12, 2024 22:47
Rewrote file to use a lambda map rather than the massive if else chain. Kept the original comments alongside each of the different scripts they were by before.
@EmosewaMC
Copy link
Collaborator

Issue #1511
Change this to Fixes #1511 so the issue is automatically marked as closed

dScripts/CppScripts.cpp Show resolved Hide resolved
script = new WildNinjaSensei();
else if (scriptName == "scripts\\ai\\WILD\\L_LUP_generic_interact.lua")
script = new LupGenericInteract();
else if (scriptName.rfind("scripts\\zone\\LUPs\\RobotCity Intro\\WBL_RCIntro_RobotCitizen", 0) == 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where did these go?

Copy link
Collaborator

Choose a reason for hiding this comment

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

there should be 4-5 with this style name we need to load, its the only missing script from the transfer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are 5, one for each color. Currently at lines 680-684 in the CPPScripts.cpp file

If this is still a question let me know

{"scripts\\ai\\WILD\\L_WILD_NINJA_SENSEI.lua", [](){return new WildNinjaSensei();}},
{"scripts\\ai\\WILD\\L_LUP_generic_interact.lua", [](){return new LupGenericInteract();}},

//And on, and on, and on, and on
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
//And on, and on, and on, and on

@EmosewaMC
Copy link
Collaborator

Please also format your files before commiting with alt+shift+f in VSCode or your preferred editor

@aronwk-aaron aronwk-aaron changed the title CppScripts Cleanup chore: CppScripts Cleanup May 14, 2024
@aronwk-aaron
Copy link
Member

You don't need to close this PR and open a new one, pushing to your branch will add changes to it

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.

ENH: Cleanup script loading
3 participants