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

Imperial stockpile round 3 #1928

Merged
merged 38 commits into from Jan 9, 2018

Conversation

@agrrr3
Copy link
Contributor

commented Dec 23, 2017

WIP for round three of the imperial stockpile.

Ratio is gone, limit is used for input as well as output.

Population of species with GOOD_ or GREAT_STOCKPILE boost the limit.

Some bonus if you switch your capital to logistics.

@dbenage-cx

This comment has been minimized.

Copy link
Member

commented Dec 24, 2017

Missing colony building/script update for added species?

@geoffthemedio

This comment has been minimized.

Copy link
Member

commented Dec 31, 2017

Meter-based stockpile limit: #1930

geoffthemedio and others added 24 commits Dec 27, 2017
-Added stockpile setting macros and related stringtable entries.
-Gave humans average stockpiling as test.
-Removed transfer limit and efficiency meters.
-Adjusted content to increase stockpile meters when using stockpile focus.

@agrrr3 agrrr3 force-pushed the agrrr3:imperial_stockpile_round_3 branch from af68220 to c4bdc77 Jan 8, 2018

@agrrr3

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2018

Rebased on geoffs Meter-based stockpile limit #1930

Added an easily researchable flux-drive like hull with a single internal slot for hidden expansion.

@@ -22,7 +22,8 @@ BuildingType
]
stackinggroup = "GAS_GIANT_GEN_STACK"
priority = [[VERY_LATE_PRIORITY]]
effects = SetTargetIndustry value = Value + 10
effects = SetTargetIndustry value = Value + 5

This comment has been minimized.

Copy link
@geoffthemedio

geoffthemedio Jan 8, 2018

Member

This might be better filtered by scope, rather than a complicated statistic in the "calculation" of 5 or 10.

This comment has been minimized.

Copy link
@agrrr3

agrrr3 Jan 8, 2018

Author Contributor

You mean duplicating the EffectGroups one for GasGiants and one for not GasGiants, right?

This comment has been minimized.

Copy link
@geoffthemedio

PRO_VOID_PREDICTION_DESC
'''While classic prediction always relies on statistical knowledge of the past, studying the chaotic wisdom of the void leads to robust predictions of first-time and freak
occurences.

This comment has been minimized.

Copy link
@geoffthemedio

geoffthemedio Jan 8, 2018

Member

odd extra newline

This comment has been minimized.

Copy link
@agrrr3

agrrr3 Jan 8, 2018

Author Contributor

freak newline occurence, ill fix

]
effects = [
SetMaxStockpile value = Value + Target.Population * [[STOCKPILE_PER_POP]] * ( 1
+ 10 * (Statistic If Condition = Focus type = "FOCUS_STOCKPILE")

This comment has been minimized.

Copy link
@geoffthemedio

geoffthemedio Jan 8, 2018

Member

why dependent on growth or logistics focus?

This comment has been minimized.

Copy link
@agrrr3

agrrr3 Jan 8, 2018

Author Contributor

The idea was to make the techs independent of the focus, just work better if you combine them.
If we dont want such a thing, PRO_GENERIC_SUPPLIES should be a prereq.

This comment has been minimized.

Copy link
@geoffthemedio

geoffthemedio Jan 8, 2018

Member

There is a focus specifically for stockpiling, and the logistics and growth focuses are for other purposes. Doesn't make sense to me to have stockpile related effects depend on them.

Focus
name = "FOCUS_STOCKPILE"
description = "FOCUS_STOCKPILE_DESC"
location = OwnerHasTech name = "PRO_GENERIC_SUPPLIES"

This comment has been minimized.

Copy link
@geoffthemedio

geoffthemedio Jan 8, 2018

Member

Need to tweak the descriptions in a few places to make it clear this tech is required. Maybe also make it a prereq for Void Prediction...?

@geoffthemedio geoffthemedio merged commit 4b45d52 into freeorion:master Jan 9, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@dbenage-cx

This comment has been minimized.

Copy link
Member

commented on default/stringtables/en.txt in be8bd38 Jan 9, 2018

Is duplicate entry, maybe intended to change METER_STOCKPILE_VALUE_LABEL or a distinct key?

This comment has been minimized.

Copy link
Member Author

replied Jan 10, 2018

probably... just fix it?

@Dilvish-fo

This comment has been minimized.

Copy link
Member

commented Jan 22, 2018

@agrrr3 in the future, please keep in mind that when that there should be a specific shout-out to the AI team calling their attention when introducing something new and significant that the AI will have to deal with, like a new species that can colonize Gas Giants (although supertesters can also use them, they are not a 'playable' race that the AI is expected to handle). Even if an AI dev noticed a new species was introduced here, they would easily assume that without any special announcement that it would be drawing entirely from the existing species characteristics that the AI can handle.

Or when changing the value of something like the gas giant generator, which the AI uses, please make a shout-out to the AI team.

In at least one spot (when valuing gas giant generators) the AI code assumes that Gas Giants are not habitable, and it seems to me that it was a bit of luck that the AI can handle the Sly as well as it can.

@freeorion/ai-team if one of you had already noticed and was working on handling the Sly, please let me know, otherwise I'll add it to my to-do list. All I see for so far that definitely needs adjustment is the handling for gas giant generators (both the assumption that it must be a diff planet than being considered for habitation, and the size of industry bonus if the gas giant is inhabited.) Also, from related discussions on the forums I see a proposal for the gas giant 'size' to change from 6 to 4, although I'm a bit doubtful about that actually happening.

@agrrr3

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2018

@Dilvish-fo

This comment has been minimized.

Copy link
Member

commented Jan 26, 2018

By flagging the pull request or how?

@agrrr3 just by using @freeorion/ai-team like I did above (and am about to do again), in the PR description or a followup message, along with a summary of what specifically you want to bring to our attention or ask about.

And how do I know if something is relevant for AI without knowing the AI code?

Ah, well, I thought you might have noticed that the AI builds Gas Giant Generators, but I understand that for many things there might be doubt (in which case the safest route is to ask). How about, in general, you provide a summary of the relevant changes that would be suitable letting an experienced player understand for their own play what the changes are. It's ideal that the players get something like that anyway without having to try scanning the pedia for changes, or reviewing all the commits, and that would also be helpful for the change-log. That level of info should be sufficient for the AI team, and if they had further questions they could ask or at least would know they need to dig through those commits in more detail.

@freeorion/ai-team I've started playing the Sly a little bit, and pretty soon I will open either an issue or a work-in-progress PR about having the AI code handle them, where I'll brainstorm some ideas

@Morlic-fo

This comment has been minimized.

Copy link
Contributor

commented Jan 26, 2018

@freeorion/ai-team if one of you had already noticed and was working on handling the Sly, please let me know, otherwise I'll add it to my to-do list.

Thanks for catching and adressing this.

I think it would be a good procedure to always open an issue in case someone notices unsupported AI stuff even when planning to adress it relatively quickly oneself (self-assign in that case).

That way, if the patch takes longer than expected and/or stuff gets in the way, it is noticed in an open issue rather than an already merged PR and someone's private todo list.

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