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

Large assortment of additions / changes including custom random side selectors #70

Merged
merged 43 commits into from May 1, 2018

Conversation

Starkku
Copy link
Contributor

@Starkku Starkku commented Apr 25, 2018

Basically a number of changes I've done for my private fork of the client or things that were suggested to me by others that I thought could be useful for a wider audience.

I will go into more detail on individual additions / changes below.

Custom Random Side Selectors

This is a fairly big one. Allows you to declare custom random side selectors that can pick random side to play as from specific set of sides rather than all. These are defined in GameOptions.ini.

Example:

[RandomSelectors]
RandomSelector1=0,1,2
RandomSelector2=3,4,5

This adds two new random selectors that can select from side indices 0, 1 and 2 or 3, 4 and 5, respectively. Icons are read in same fashion as for other sides (f.ex, first of the above selectors would read randomselector1icon.png).

Duplicate selector names are not allowed, only first one is added. Duplicate sides in selector listing, or sides that exceed bounds of available side indices are excluded. Selector is not added if it has 1 or less valid side indices listed. Currently replicating the functionality of the built-in random selector is allowed but serves little purpose. Interaction with co-op map disallowed side logic should also work properly, disabling the selector if only one or less of its associated sides are available.

Potential Use Cases: Most obvious one is allowing side-specific random selectors for mods that use subfactions.

New Map / Game Mode Features

Specifically this adds following flags to both maps and gamemodes.

  • ForceRandomStartLocations - Forces no preset starting locations and all players to start at random locations. Not available for IsCoopMission=yes maps.
  • ForceNoTeams - Forces no preset teams. Not available for IsCoopMission=yes maps.
  • HumanPlayersOnly - Disables AI players. Only available if current gamemode or map is MultiplayerOnly=yes.

Potential Use Cases: These came as a suggestion from a modder. There are certain types of game modes or maps that could benefit from these. Disabling AI and teams are also features that were present in original RA2/YR.

Game Lobby Option Changes / Additions

There is a new DataWriteMode for dropdowns called MapCode. This DataWriteMode does not write any values to spawner INI, but instead allows including map INI code based on selected item. Items should be INI filenames, relative to game root directory

Example:

[cmbTimeOfDay]
OptionName=Time of Day
Items=INI\Game Options\Day.ini,INI\Game Options\Night.ini
DataWriteMode=MapCode

Above feature is supplemented by addition of new flag ItemLabels which is parsed as a comma-separated list of strings to use as overrides for the displayed text for dropdown items if DataTypeWrite is set to String or MapCode. If there are not enough labels listed to override text for all listed items, the remaining items will display their original text. Excess labels listed beyond the number of available items are left unused.

Example:

[cmbTimeOfDay]
ItemLabels=Day,Night

Implementation of ItemLabels is not perfect considering parsing this flag deviates from the pattern rest use, this is to ensure that Items and ItemLabels can be listed in any order.

Also, when writing values to spawner INI or including map INI code, if dropdown has no valid item selected (invalid index used in ForcedOptions, for an example), a crash is no longer produced but writing the associated value is instead skipped entirely.

Finally, in the INI files associated with game lobby option checkboxes or dropdowns, it is possible to now define additional INI files to include only if a specific game mode is selected. The included files are merged to map file after the parent, so they can be used to override code in there.

Example:

[GameModeIncludes]
Standard=INI\Game Options\GameOptionX-Standard.ini
Cooperative=INI\Game Options\GameOptionX-Cooperative.ini

Potential Use Cases: Map code dropdowns is an another feature suggested by a modder. Besides the example used where it is used to apply cosmetic lighting effects on maps, there is potential for more gameplay-influencing features like customizable timer lengths and such.

ItemLabels was a necessity for the map code dropdowns, and it can also be used in conjunction with existing dropdowns like Tech Level to convert it from more fine-grain control to only specific tiers of technology (WF, Radar etc).

Preventing crash from invalid selected item indices is a hacky solution to allow dropdowns to be blanked out on specific maps / gamemodes, writing no values or map code. Proper gamemode-specific dropdowns / checkboxes would require significantly more work to achieve.

GameModeIncludes is something I personally wanted to implement to make the commonly-used option of 'brutal / cheating AI' more adaptable to specific rules of certain game modes such as naval-based combat. I am sure there are other potential uses for it, as well.

FileHashCalculator Customization

Simply put, it is now possible to override the list of files checked when generating the hashes by providing a list of files in new INI file called FHCConfig.ini in Resources directory.

Example:

[FilenameList]
file1.ini
Resources\thing.exe

Listing any filenames on this list will override the entire built-in list. INI\GlobalCode.ini and FHCConfig.ini itself are always checked regardless of if FHCConfig.ini exists or is used or not.

Potential Use Cases: Some mods, particularly those that use Ares might want to include files in this check that are not present on any of the built-in lists, such as INI files associated with Ares' [#include] functionality.

Skirmish Setting Loading Changes / Additions

These changes mostly pertain to more reliable loading of skirmish settings. Of particular note are player starting locations, which instead of only being loaded for first X players, where X is the maximum player count of first map in the map list they are now reliably applied to all players, thanks to gamemode and map being loaded earlier. In addition, manually tweaking the SkirmishSettings.ini produces less unusual / unintended behaviour like forcing AI to non-existing spectator side slot or using a side disabled on the map.

It is also now possible to enable saving the values of game option checkboxes / dropdowns, -by enabling setting SaveSkirmishGameOptions in ClientDefinitions.ini.

Disabling Certain UI Features

Following settings were added to ClientDefinitions.ini:

  • DisableMultiplayerGameLoading - If set, removes the Load Game buttons for multiplayer. The then-vestigial dialog when creating a LAN game will also be skipped.
  • DisableComponentOptions - If set, removes the Component panel in options.
  • DisableUpdaterOptions - If set, removes the Updater and Components panels in options.

Top Bar Changes / Additions

The time display on top bar is now always displayed in the same, unlocalized format. Previously on certain locales the time would be formatted in a way that the display would get cut off due to lack of space.

It is also now possible to move the 'players online' counter from main menu to the top bar by enabling DisplayPlayerCountInTopBar in ClientDefinitions.ini. This disables the counter in main menu, although the associated labels are left intact without the logic that updates the text.

Clear Statistics Database Button

Added a button (Name: btnClearStatistics) to StatisticsWindow, which if clicked prompts a confirmation dialog. If accepted, current statistics database is wiped clean. This button is not visible by default and must be made visible in an appropriate INI file before it can be used.

Changes to Permissions Check

Client now performs a separate check to more reliably determine if it is being ran from Program Files without administrator privileges - something that can and will cause problems. If this is the case, it prompts restart with the said privileges.

Miscellanneous Changes

  • Game names on hosted games list are no longer truncated to make room for '(Loaded Game)' suffix unless the game in question is a loaded game.
  • Default position for player start location dropdown in game lobby now defaults to being placed next to team dropdown on right-hand side instead of overlapping each other.
  • Allow automatic creation of saved games directory if it is missing on client startup by enabling CreateSavedGamesDirectory in ClientDefinitions.ini
  • Setting UnixMapEditorExePath in ClientDefinitions.ini can now be used to define separate executable for map editor on Unix systems.
  • Added yet another missing YR hotkey (Delete Beacon) omitted previously.
  • Several controls in StatisticsWindows were missing names making them unaccessible to the INI system, this has been fixed.
  • CnCNet lobby & related code no longer assumes specific format for chat & broadcast channel names (#cncnet-gameId[-games]), but rather always attempts to fetch the channel names from the game list defined in game collection. This is a backend change for consistency and to help people who possibly want to make a build of the client to support a new mod, allowing them to only edit game collection code and ensuring that they can use registered channel names more easily as names containing 'cncnet' are protected by GameSurge and not registered unless registrant is CnCNet staff.

…m a file called FHCConfig.ini in Resources directory.
…eges under Program Files, prompting restart with the said elevated privileges.
# Conflicts:
#	ClientCore/ClientCore.csproj
…ame list if the game actually is loaded game.
…s AI players from using a non-existing spectator side option.
… with DataWriteMode=String by listing ItemLabels.
@Rampastring
Copy link
Member

Most of these changes sound great. I'll review the code later on.

@Rampastring
Copy link
Member

Some things:

The time display on top bar is now always displayed in the same, unlocalized format. Previously on certain locales the time would be formatted in a way that the display would get cut off due to lack of space.

I prefer the localized format. If there's some locales where it's problematic right now, I'd rather dynamically calculate the size of the time label and move it so it's always perfectly visible and aligned.

It is also now possible to move the 'players online' counter from main menu to the top bar by enabling DisplayPlayerCountInTopBar in ClientDefinitions.ini. This disables the counter in main menu, although the associated labels are left intact without the logic that updates the text.

Making it possible to display the counter in the top bar is a good idea. Is there a reason why it should disable the counter in the main menu, though? I think the modder should be able to have both, if they wish so. The main menu counter can be already be hidden by editing MainMenu.ini.

Client now performs a separate check to more reliably determine if it is being ran from Program Files without administrator privileges - something that can and will cause problems. If this is the case, it prompts restart with the said privileges.

The current check is meant to work for any folder, not just Program Files, although it doesn't seem to work reliably. I'd rather find a perfect solution for any folder instead of hard-coding a check for Program Files. That being said, this is an acceptable work-around until a perfect way to check for permissions is found (assuming one exists).

Otherwise the changes sound nice. I also took a quick look at the code and most of it seemed well written. There were a few things that I think could be improved, though. I'll comment on them in more detail later, or I might accept the pull request and change them myself afterwards.

@Starkku
Copy link
Contributor Author

Starkku commented May 1, 2018

I restored the localized time format and allowed main menu & top bar player counters to display simultaneously.

Whether or not you want to implement any further improvements yourself or describe them in more detail for me to work on is your call.

@Rampastring Rampastring merged commit 61be108 into CnCNet:master May 1, 2018
@Rampastring
Copy link
Member

Rampastring commented May 1, 2018

@Starkku I made some slight code changes in this commit: b3b3497

Some of the changes are up to personal preferences, while others are more "logical".

  • Avoid having one-line if statements
  • Prefer private or protected instead of the assembly-wide internal
  • Make use of the expression body for methods introduced in C# 6 when possible, like in pure getters (there's some old code in the client that doesn't make use of it yet, but new code should still use modern C# features)
  • The INI class doesn't allow duplicate keys, so there's no point in calling Distinct() when iterating through the keys returned by GetSectionKeys
  • Avoid catching all exceptions, catch specific exceptions instead at least if we only have some specific exception type to expect from an operation, like a FormatException when converting a string to an int (I know a lot of my own code, particularly older code, violates this a lot, but still)
  • No need to use Where when we are trying to find a single item from a collection, Where builds a whole new collection. In case of reference types, prefer Find, for structs like the KeyValuePair used in skirmish setting loading, FindIndex works
  • Use camelCase when naming variables
  • Avoid writing very long lines. Try to keep lines around 80 characters at max

But yeah, nice job, I bet a lot of YR modders in particular will find these useful 👍

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.

None yet

2 participants