Skip to content

Station rename command#984

Merged
AaronVanGeffen merged 7 commits intoOpenLoco:masterfrom
AaronVanGeffen:gc/station-name
Jul 1, 2021
Merged

Station rename command#984
AaronVanGeffen merged 7 commits intoOpenLoco:masterfrom
AaronVanGeffen:gc/station-name

Conversation

@AaronVanGeffen
Copy link
Copy Markdown
Member

No description provided.

This re-introduces the possibilty of resetting/regenerating station names automatically
by using an empty station name. Previously, our reimplementation (C++ code) inadvertently
removed this option.
@AaronVanGeffen AaronVanGeffen added the changelog Requires a changelog entry label Jun 28, 2021
@AaronVanGeffen AaronVanGeffen marked this pull request as ready for review June 28, 2021 13:47
@AaronVanGeffen
Copy link
Copy Markdown
Member Author

Updated the PR to remove the commits implementing StationManager::generateNewStationName. This is now ready for review.

// If an empty string is given, generate one instead.
if (strlen(renameStringBuffer) == 0)
{
// Bailing out early?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this a question. It is bailing out early as we are not applying but everything is fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because we're still 'asking' at that point, by means of the if block. It's just my comment style. We can change it, if you like.

if (callingWidget != Common::widx::caption)
return;

if (strlen(input) == 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it safe to do the game command with an empty string? Guess its just outputting memory that is ignored. Perhaps we should add a comment.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If the input string is empty, a name will be generated instead.

Copy link
Copy Markdown
Contributor

@duncanspumpkin duncanspumpkin left a comment

Choose a reason for hiding this comment

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

All looks fine. I wonder if we could reduce duplication though.

@AaronVanGeffen AaronVanGeffen merged commit fbc145a into OpenLoco:master Jul 1, 2021
@AaronVanGeffen AaronVanGeffen deleted the gc/station-name branch July 1, 2021 17:59
tomasharkema added a commit to tomasharkema/OpenLoco that referenced this pull request Jul 13, 2021
…ature/mac-docker

* 'master' of https://github.com/OpenLoco/OpenLoco: (536 commits)
  Mention code style in readme (OpenLoco#1047)
  Remove redundant rotate function (OpenLoco#1046)
  Link up implemented function (OpenLoco#1043)
  Fix use of constants for coord limits (OpenLoco#1037)
  validCoords: remove template parameter and name type (OpenLoco#1033)
  Add coordinate validation to tile loops with offsets (OpenLoco#1031)
  Implement ChangeCompanyColour game command (OpenLoco#1029)
  Implement colour picker dropdowns (OpenLoco#1028)
  Implement StationManager::generateNewStationName (OpenLoco#1020)
  Station rename command (OpenLoco#984)
  Fix access to embedded object name (OpenLoco#1022)
  Reduce inclusion of StringIds.h in header files (OpenLoco#985)
  Implement the station name background paint (OpenLoco#1023)
  Force alignment on TileManager::createAnimation hook
  Merge duplicated flags and use accessors (OpenLoco#1025)
  Remove use of global stringformatbuffer where not required (OpenLoco#1024)
  Refactor calls to tryCreateInitialMovementSound (OpenLoco#1018)
  Turn widget draw functions into Widget struct member functions (OpenLoco#1012)
  Allow filtering the vehicle list by station or cargo (OpenLoco#997)
  Restore game command table alignment
  ...
@AaronVanGeffen AaronVanGeffen added this to the v21.07 milestone Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog Requires a changelog entry

Projects

No open projects
Status: Done (PRs)

Development

Successfully merging this pull request may close these issues.

2 participants