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

Feature: Allow GameScripts to add additional text to Industry view window #8576

Merged
merged 1 commit into from Jan 22, 2021

Conversation

ldpl
Copy link
Contributor

@ldpl ldpl commented Jan 15, 2021

Motivation / Problem

With #8115 and #7912 been merged it becomes quite important for GameScripts to have the ability to communicate some extra info about specific industries to the player. This PR adds an additional text field to the industry that is shown at the bottom of the industry view window, much like it's already done for towns.

GS for testing:
industry-text-test.zip

Checklist for review

  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.

src/saveload/saveload.h Outdated Show resolved Hide resolved
Copy link
Member

@LordAro LordAro left a comment

No real concerns

src/script/api/script_error.hpp Outdated Show resolved Hide resolved
src/industry.h Outdated Show resolved Hide resolved
@ldpl ldpl force-pushed the industry-text branch 2 times, most recently from 1b700e8 to 6c42ea2 Compare Jan 16, 2021
@michicc
Copy link
Member

@michicc michicc commented Jan 17, 2021

Note for text in the comment of CmdIndustryCtrl is now outdated.

SetControlFlags = 0, ///< Set IndustryControlFlags
SetExclusiveSupplier = 1, ///< Set exclusive supplier
SetExclusiveConsumer = 2, ///< Set exclusive consumer
SetText = 3, ///< Set additional text
Copy link
Member

@michicc michicc Jan 18, 2021

Choose a reason for hiding this comment

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

Coding style for enum members:

LordAro: obviously it doesn't need the prefix, but yeah, should probably still be all caps

Copy link
Contributor Author

@ldpl ldpl Jan 18, 2021

Choose a reason for hiding this comment

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

Other 2 enum classes (ElementFloat and MidiSysexMessage) use this syntax. And IMO it's much nicer, as they are not in global namespace THERE IS NO NEED TO SHOUT :p

Copy link
Member

@michicc michicc Jan 18, 2021

Choose a reason for hiding this comment

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

Well, letting @LordAro have the last word here, but otherwise 👍

@LordAro LordAro merged commit bab7de6 into OpenTTD:master Jan 22, 2021
8 checks passed
@ldpl ldpl deleted the industry-text branch Sep 19, 2021
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

4 participants