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

Convar print commands #539

Merged
merged 13 commits into from
Nov 5, 2023
Merged

Conversation

H0L0theBard
Copy link
Contributor

@H0L0theBard H0L0theBard commented Sep 4, 2023

Adds various concommands such as

  • convar_findByFlags
  • convar_list
  • convar_differences
  • convar_find

The first 3 listed above are already registered as concommands natively but didn't seem to do anything when tested.
convar_findByFlags and convar_find were already implemented by Bob under the names findflags and find respectively but the names have been changed to reflect already existing convars.

All outputs into the console are also alphabetically sorted.
image

Changes:
- Rename find to convar_find to keep naming similar to other Convar printing commands
- Rename findflags to convar_findByFlags as this command already exists in the engine
-  Implements  convar_list as it already exists in the engine
- Implements convar_differences as it already exists in the engine

Issues:
- convar_list is currently not sorted in alphabetical order
- convar_differences will likely need a formatting change
Changes:
- Implements alphabetically sorted outputs from concommands
- Format the convar_differences output
@ASpoonPlaysGames ASpoonPlaysGames 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 labels Sep 4, 2023
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

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

Code looks good!

@ASpoonPlaysGames ASpoonPlaysGames removed the needs code review Changes from PR still need to be reviewed in code label Sep 4, 2023
@cyrv6737
Copy link

cyrv6737 commented Sep 5, 2023

Works on My Machine

@Asha097
Copy link

Asha097 commented Sep 5, 2023

I tested this and now want to implode (It works)

@Goose645
Copy link

Goose645 commented Sep 5, 2023

works (no crashes for 30 min)

Copy link
Member

@catornot catornot left a comment

Choose a reason for hiding this comment

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

the code looks good, I just have a nitpick.

NorthstarDLL/util/printcommands.cpp Outdated Show resolved Hide resolved
@H0L0theBard
Copy link
Contributor Author

Going to be reworking the code to use g_pCVar->FactoryInternalIterator() and fixing minor issues which are still in this pr

@Alystrasz
Copy link
Contributor

@H0L0theBard 'sup?

enforces convars with devonly and hidden flags not being shown by default

introduces a bit of code duplication but this is what's in the engine anyway
@cyrv6737
Copy link

PR still working for me after 00bab5d

@Asha097
Copy link

Asha097 commented Sep 19, 2023

Tested again and still want to explode (works)

@itscynxx
Copy link
Contributor

was able to run all listed commands with proper outputs and no issues 👍

@ASpoonPlaysGames ASpoonPlaysGames added READY TO MERGE This mergeable right now needs code review Changes from PR still need to be reviewed in code and removed needs testing Changes from the PR still need to be tested READY TO MERGE This mergeable right now labels Sep 19, 2023
@ASpoonPlaysGames
Copy link
Contributor

(Whoops, missed that code needs re-reviewing since changes)

Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

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

Minor change and a question

NorthstarDLL/util/printcommands.cpp Outdated Show resolved Hide resolved
NorthstarDLL/util/printcommands.cpp Outdated Show resolved Hide resolved
@H0L0theBard
Copy link
Contributor Author

I just realised that convar_list (cvarlist in valve source) has the ability to make a list from a search and log to a file.

cvarlist mat_ - creates a cvarlist of all "mat_" commands
cvarlist log allconvars.txt - logs cvarlist to allconvars.txt in game directory
and also convar_findByFlags is missing an autocomplete callback.

Probably should implement these eventually.
Wouldn't really change much so I can perhaps do it in another pr as the basic functionality is added in this one

- convar_differences not as nested
- convar_list prints total amount
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

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

Code looks good, requested changes were addressed :)

@itscynxx
Copy link
Contributor

itscynxx commented Oct 5, 2023

Was able to open the console and run all commands with proper outputs with the newest version 👍

@GeckoEidechse GeckoEidechse removed the needs code review Changes from PR still need to be reviewed in code label Oct 19, 2023
@ASpoonPlaysGames ASpoonPlaysGames added the READY TO MERGE This mergeable right now label Oct 27, 2023
@ASpoonPlaysGames
Copy link
Contributor

seems that @GeckoEidechse forgot to add the "ready to merge" label when removing "needs code review"

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

Seems to be working fine in quick testing.

@GeckoEidechse GeckoEidechse merged commit 2b269d2 into R2Northstar:main Nov 5, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
READY TO MERGE This mergeable right now
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

10 participants