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

Pedia/Techs Imperial Stockpile techs cleanup #1882

Conversation

@alleryn
Copy link
Contributor

commented Nov 27, 2017

Categorize the Imperial Stockpile tech pedia entries, move Transcendental Design from Construction to Production for consistency with other Imperial Stockpile techs, rename the focs files to match the tech names.

Forum post: http://www.freeorion.org/forum/viewtopic.php?f=28&t=10776

@dbenage-cx
Copy link
Member

left a comment

Stringtable keys should align with definition filenames.
(X.focs.txt -> name= "X" description = "X_DESC")

As already being moved, would be nice to have a new article to group these techs under (lmk if you need an example). If you would rather not touch these yet, outside of filenames, LGTM.

@alleryn

This comment has been minimized.

Copy link
Contributor Author

commented Nov 28, 2017

would be nice to have a new article to group these techs under

You mean something like a sub-category in the Pedia right? I'd like to do a pass to add another level to all the Pedia/Technologies/x categories next week or so. Seems better than having just the Imperial Stockpile techs at level 2, with alll the other techs at level 1.


As a sidenote, i noticed that the solar orbital generator focs file has a slightly different filename from its keys (SOLAR_ORB_GEN vs SOL_ORB_GEN). Seems minor and outside the scope of this PR, so i just let it be.

@dbenage-cx

This comment has been minimized.

Copy link
Member

commented Nov 29, 2017

Missing change for starting_unlocks/items.inf

Pedia/Technologies/x categories

Think the only other that might benefit are Defense and Intelligence/Spy. As that is for other work, will post a fuller response next chance I can log into forum.

re: SOLAR_ORB_GEN, probably close enough unless there are other changes needed for that file, at least until some code or policy change requires stricter naming. Personally was only concerned with completely different naming between file and definition names (X_1.focs.txt containing name = "OTHER"), though some wholesale cleanup might be warranted.

@alleryn

This comment has been minimized.

Copy link
Contributor Author

commented Nov 29, 2017

Good work catching my error on starting unlocks. I've learned the grep command now, so hopefully such oversights will be less likely in the future.

I started on the forum with my ideas for subcategorization: http://www.freeorion.org/forum/viewtopic.php?f=6&t=10343&start=15#p90749 (it's a w.i.p.)

@dbenage-cx

This comment has been minimized.

Copy link
Member

commented Nov 30, 2017

Can you squash/fixup (see rebase) the overlapping commits?

An example order might be: git rebase -i HEAD~7

pick c2e8fb3af Change Trans Design's Tech Caegory to Prod
fixup ea4ed9aff
pick 49b18145c Add Imperial Stockpile Techs to Pedia/Techs/Production
pick fb2769a88 Rename ImperialStockpileX focs files
fixup 39343bdfe
fixup ec8322727
pick 151ae6bfa Fix typo in stringtables TRANS_DESIGN_DESC
@alleryn

This comment has been minimized.

Copy link
Contributor Author

commented Nov 30, 2017

pick c2e8fb3af Change Trans Design's Tech Caegory to Prod
fixup ea4ed9aff
pick 49b18145c Add Imperial Stockpile Techs to Pedia/Techs/Production
pick fb2769a88 Rename ImperialStockpileX focs files
fixup 39343bdfe
fixup ec8322727
pick 151ae6bfa Fix typo in stringtables TRANS_DESIGN_DESC

Should 'fixup ea4ed9a' be merged into the "Rename ImperialStockpileX' commit, along with the other two fixups?

ea4ed9a is the starting unlock fix which was a problem with PRO_IMPERIAL_STOCKPILE_1 vs PRO_PREDICTIVE_STOCKPILING. It seems unrelated to the change in Trans Design's Tech Category.

Does this order look okay?

reword c2e8fb3af Change Trans Design's Tech Caegory to Prod
pick 49b18145c Add Imperial Stockpile Techs to Pedia/Techs/Production
pick fb2769a88 Rename ImperialStockpileX focs files
fixup 39343bdfe
fixup ec8322727
fixup ea4ed9aff
pick 151ae6bfa Fix typo in stringtables TRANS_DESIGN_DESC
@dbenage-cx

This comment has been minimized.

Copy link
Member

commented Nov 30, 2017

Sounds good!

@alleryn alleryn force-pushed the alleryn:Pedia_Techs_ImperialStockpileTechs_Cleanup branch from ea4ed9a to ac77ed3 Nov 30, 2017

@dbenage-cx dbenage-cx merged commit c1af76a into freeorion:master Dec 1, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dbenage-cx dbenage-cx self-assigned this Dec 1, 2017

@dbenage-cx

This comment has been minimized.

Copy link
Member

commented Dec 1, 2017

@alleryn Thanks for addressing these, preferred name for credits?

@alleryn

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2017

preferred name for credits?

Timothy Jaeger

Thanks for all your help, dbenage-cx!

@Vezzra

This comment has been minimized.

Copy link
Member

commented Dec 1, 2017

FYI: I just added @alleryn to the credits.

@alleryn alleryn deleted the alleryn:Pedia_Techs_ImperialStockpileTechs_Cleanup branch Dec 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.