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.

Sign up for GitHub

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

Jump to bottom

Add script concommands #373

Merged
merged 9 commits into from
Dec 22, 2022
Merged

Add script concommands #373

merged 9 commits into from
Dec 22, 2022

Conversation

emma-miler
Copy link
Member

@emma-miler emma-miler commented Dec 17, 2022 •
edited by GeckoEidechse
Loading

Example:

"ConCommands": [
	{
	  "Name": "concommand_test",
	  "Function": "ConCommandTest",
	  "Context": "UI",
	  "Flags": 0,
	  "HelpString": "example script concommand"
	}
  ],

Erlite
Erlite suggested changes Dec 17, 2022
View reviewed changes
Copy link
Contributor

@Erlite Erlite left a comment

Choose a reason for hiding this comment

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

Few formatting nitpicks, and potentially one logic issue with the switch statement.

NorthstarDLL/modmanager.cpp Outdated Show resolved Hide resolved
NorthstarDLL/modmanager.cpp Outdated Show resolved Hide resolved
NorthstarDLL/modmanager.cpp Outdated Show resolved Hide resolved
NorthstarDLL/modmanager.cpp Outdated Show resolved Hide resolved
NorthstarDLL/modmanager.cpp Outdated Show resolved Hide resolved
@emma-miler emma-miler requested review from Erlite, BobTheBob9 and pg9182 and removed request for Erlite December 19, 2022 19:39
@emma-miler emma-miler added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Dec 20, 2022
BobTheBob9
BobTheBob9 requested changes Dec 20, 2022
View reviewed changes
Copy link
Member

@BobTheBob9 BobTheBob9 left a comment

Choose a reason for hiding this comment

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

looks great, just needs adjustments

NorthstarDLL/mods/modmanager.cpp Show resolved Hide resolved
NorthstarDLL/mods/modmanager.cpp Show resolved Hide resolved
NorthstarDLL/mods/modmanager.cpp Show resolved Hide resolved
@emma-miler emma-miler requested review from BobTheBob9 and removed request for Erlite December 20, 2022 22:56
@emma-miler emma-miler merged commit 6410006 into R2Northstar:main Dec 22, 2022
@Glacir Glacir mentioned this pull request Apr 8, 2023
Merged
pg9182 added a commit to pg9182/NorthstarLauncher that referenced this pull request Apr 19, 2023
@pg9182
Fix parsing string ConVar/ConCommand.Flags from mod.json
94c2f69 
The ParseConVarFlagsString function introduced in 6410006 (R2Northstar#373)
is utterly broken. It only parses the first flag, logs misleading
warnings, has an undefined return value in some codepaths, and is
somewhat convoluted.

Luckily, this doesn't appear to affect most (if not all) existing mods,
as they all seem to be using integer values for Flags, which is
taken as-is.

https://github.com/search?q=path%3A**%2Fmod.json+ConVars+Flags&type=code
@pg9182 pg9182 mentioned this pull request Apr 19, 2023
Merged
pg9182 added a commit to pg9182/NorthstarLauncher that referenced this pull request Apr 19, 2023
@pg9182
Fix parsing string ConVar/ConCommand.Flags from mod.json
21f2bbf 
The ParseConVarFlagsString function introduced in 6410006 (R2Northstar#373)
is utterly broken. It only parses the first flag, logs misleading
warnings, has an undefined return value in some codepaths, and is
somewhat convoluted.

Luckily, this doesn't appear to affect most (if not all) existing mods,
as they all seem to be using integer values for Flags, which is
taken as-is.

https://github.com/search?q=path%3A**%2Fmod.json+ConVars+Flags&type=code
@GeckoEidechse GeckoEidechse removed needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Apr 19, 2023
pg9182 added a commit that referenced this pull request Apr 19, 2023
@pg9182
Fix parsing string ConVar/ConCommand.Flags from mod.json (#450)
34af81a 
Fix parsing string ConVar/ConCommand.Flags from mod.json

The ParseConVarFlagsString function introduced in 6410006 (#373)
is utterly broken. It only parses the first flag, logs misleading
warnings, has an undefined return value in some codepaths, and is
somewhat convoluted.

Luckily, this doesn't appear to affect most (if not all) existing mods,
as they all seem to be using integer values for Flags, which is
taken as-is.

https://github.com/search?q=path%3A**%2Fmod.json+ConVars+Flags&type=code
GeckoEidechse pushed a commit that referenced this pull request Apr 19, 2023
@pg9182 @GeckoEidechse
Fix parsing string ConVar/ConCommand.Flags from mod.json (#450)
b019e0b 
Fix parsing string ConVar/ConCommand.Flags from mod.json

The ParseConVarFlagsString function introduced in 6410006 (#373)
is utterly broken. It only parses the first flag, logs misleading
warnings, has an undefined return value in some codepaths, and is
somewhat convoluted.

Luckily, this doesn't appear to affect most (if not all) existing mods,
as they all seem to be using integer values for Flags, which is
taken as-is.

https://github.com/search?q=path%3A**%2Fmod.json+ConVars+Flags&type=code
wolf109909 pushed a commit to R2NorthstarCN/NorthstarLauncher that referenced this pull request Apr 22, 2023
@pg9182 @wolf109909
Fix parsing string ConVar/ConCommand.Flags from mod.json (R2Northst…
80bc558 
…ar#450)

Fix parsing string ConVar/ConCommand.Flags from mod.json

The ParseConVarFlagsString function introduced in 6410006 (R2Northstar#373)
is utterly broken. It only parses the first flag, logs misleading
warnings, has an undefined return value in some codepaths, and is
somewhat convoluted.

Luckily, this doesn't appear to affect most (if not all) existing mods,
as they all seem to be using integer values for Flags, which is
taken as-is.

https://github.com/search?q=path%3A**%2Fmod.json+ConVars+Flags&type=code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Erlite Erlite Erlite requested changes

@BobTheBob9 BobTheBob9 BobTheBob9 approved these changes

@pg9182 pg9182 Awaiting requested review from pg9182

Assignees
No one assigned
Labels
None yet
Projects
Northstar Roadmap
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@emma-miler @Erlite @BobTheBob9 @GeckoEidechse