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.

Sign up for GitHub

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

Jump to bottom

Refactor ResourcesAI.py #580

Merged
merged 19 commits into from
May 12, 2016
Merged

Refactor ResourcesAI.py #580

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
bc55b40
Refactor set_planet_research_foci calling 5 new function calls
LGM-Doyle Mar 25, 2016
dc85453
Refactor set_planet_industry_and_foci
LGM-Doyle Mar 25, 2016
0e8bd8b
Fix overly indented code in set_planet_industry_and_foci.
LGM-Doyle Apr 2, 2016
8f7a0be
Move setting newFoci out of print report section of code
LGM-Doyle Mar 28, 2016
ec45191
Refactor. Restyle several ResourcesAI.py scope variables to lower_cas…
LGM-Doyle Mar 29, 2016
8bc5c8d
Created PlanetFocusManager in ResourcesAI.py
LGM-Doyle Mar 29, 2016
61f15a6
In ResearchAI.py created a focus_manager.set_future_focus function
LGM-Doyle Mar 29, 2016
df4c1ba
Made ResourcesAI.py use focus information objects
LGM-Doyle Mar 30, 2016
9fc7ce5
Created ResourcesAI.Reporter object to handle reporting
LGM-Doyle Mar 31, 2016
0ed4238
In ResourcesAI.py fixed hanging indents
LGM-Doyle Mar 31, 2016
6e63715
Compute values for report in reporting function
LGM-Doyle Apr 1, 2016
7be979e
Code review fixes from PR #580
LGM-Doyle Apr 3, 2016
b6122a6
Changed ResourcesAI.PlanetFocusInfo.current_output to a tuple
LGM-Doyle Apr 3, 2016
6317426
Added ResourcesAI.PlanetFocusInfo.can_change_focus()
LGM-Doyle Apr 3, 2016
c8cd5d2
More review changes from PR # 580
LGM-Doyle Apr 3, 2016
ec4ddbd
PEP8 for PR #580 review
LGM-Doyle Apr 4, 2016
acb78b9
Remove unused bool do_research from ResourcesAI.py
LGM-Doyle Apr 9, 2016
1920fdc
Replace PlanetInfo.can_change_focus() with PlanetInfo.planet.availble…
LGM-Doyle Apr 9, 2016
dbe6a1e
Code clarity changes to ResourcesAI.py for PR 580 review
LGM-Doyle Apr 24, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 27 additions & 27 deletions default/AI/ResourcesAI.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
transfers the planet from the raw list to the baked list, until all planets
have their future focus decided.
"""
#Note: The algorithm is not stable with respect to pid order. i.e. Two empire with
# exactly the same colonies, but different pids may make different choices.
# Note: The algorithm is not stable with respect to pid order. i.e. Two empire with
# exactly the same colonies, but different pids may make different choices.

import freeOrionAIInterface as fo # pylint: disable=import-error
import FreeOrionAI as foAI
Expand All @@ -23,15 +23,15 @@

resource_timer = Timer('timer_bucket')

#Local Constants
# Local Constants
IFocus = FocusType.FOCUS_INDUSTRY
RFocus = FocusType.FOCUS_RESEARCH
MFocus = FocusType.FOCUS_MINING # not currently used in content
GFocus = FocusType.FOCUS_GROWTH
PFocus = FocusType.FOCUS_PROTECTION
fociMap = {IFocus: "Industry", RFocus: "Research", MFocus: "Mining", GFocus: "Growth", PFocus: "Defense"}

#TODO use the priorityRatio to weight
# TODO use the priorityRatio to weight
RESEARCH_WEIGHTING = 2.0

useGrowth = True
Expand All @@ -45,8 +45,8 @@ class PlanetFocusInfo(object):
def __init__(self, planet):
self.planet = planet
Copy link
Contributor

Choose a reason for hiding this comment

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

Most of the python code takes ids instead of actual objects, probably we should do so here as well? @Cjkjvfnby opinion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used planet instead of pid to avoid the cost of a fo.getUniverse() call in every PlanetFocusInfo every game turn. This way fo.getUniverse() is called once per turn in PlanetFocusManager().

I didn't profile it, so it is probably premature optimization.
Changing it is trivial.

Should I change it?

Copy link
Contributor

Choose a reason for hiding this comment

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

For me fo.planet instead of 'int' looks ok.

  • we are sure that we don't get any wrong id. I hate that constant check fo "NonePointerException"
  • we cannot save this into savegame

self.current_focus = planet.focus
self.current_output = (planet.currentMeterValue(fo.meterType.industry)
, planet.currentMeterValue(fo.meterType.research))
self.current_output = (planet.currentMeterValue(fo.meterType.industry),
planet.currentMeterValue(fo.meterType.research))
self.possible_output = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is weird for me (eventhough consistend with legacy code). From current_output[Focus] I would expect the output (probably both research and production) if the planet has the Focus activated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

current_output is only for the current focus.

However, I would like to change from the legacy code to a tuple as that is more consistent with the treatment of possible_output which store tuples.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the point. Why does current_output have both industry and research focus as key if focus isn't a consideration.
Changing to tuple definitely makes sense to me.

itarget = planet.currentMeterValue(fo.meterType.targetIndustry)
rtarget = planet.currentMeterValue(fo.meterType.targetResearch)
Expand All @@ -55,8 +55,8 @@ def __init__(self, planet):

def can_change_focus(self):
"""Can the planet change focus? Is it's population non zero?"""
#TODO Could this be changed to current population instead of target population?
#It uses the target population to avoid situations where an unsustainable population is dying?
# It uses the target population versus current population to avoid situations
# where an unsustainable population is dying?
Copy link
Member

Choose a reason for hiding this comment

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

It's my recollection that a while in the past, when a colony died down to an outpost it still retained its last-set focus, which could not be changed, hence a test like the one here (though I don't recall why it would look at target pop rather than current pop). Nowadays the focus is reset to nothing in that case, so I think this test would no longer be needed (or perhaps simply be changed to look for nonempty AvailableFoci).

return self.planet.currentMeterValue(fo.meterType.targetPopulation) > 0


Expand Down Expand Up @@ -111,7 +111,7 @@ def calculate_planet_infos(self, pids):
It excludes baked planets from consideration.
Note: The results will not be strictly correct if any planets have global effects
"""
#TODO this function depends on the specific rule that off-focus meter value are always the minimum value
# TODO this function depends on the specific rule that off-focus meter value are always the minimum value
universe = fo.getUniverse()
unbaked_pids = [pid for pid in pids if not pid in self.baked_planet_info]
planet_infos = [(pid, self.all_planet_info[pid], self.all_planet_info[pid].planet) for pid in unbaked_pids]
Expand Down Expand Up @@ -177,8 +177,8 @@ def print_resource_ai_header():
@staticmethod
def print_table_header():
print "==================================="
print Reporter.table_format % ("Planet", "current RP/PP", "old target RP/PP"
, "current Focus", "newFocus", "new target RP/PP")
print Reporter.table_format % ("Planet", "current RP/PP", "old target RP/PP",
"current Focus", "newFocus", "new target RP/PP")

def print_table_footer(self, priorityRatio):
current_industry_target = 0
Expand Down Expand Up @@ -207,18 +207,18 @@ def print_table_footer(self, priorityRatio):

print "-----------------------------------"
print "Planet Focus Assignments to achieve target RP/PP ratio of %.2f from current ratio of %.2f ( %.1f / %.1f )" \
% (priorityRatio, current_research_target / (current_industry_target + 0.0001)
, current_research_target, current_industry_target)
% (priorityRatio, current_research_target / (current_industry_target + 0.0001),
current_research_target, current_industry_target)
print "Max Industry assignments would result in target RP/PP ratio of %.2f ( %.1f / %.1f )" \
% (all_industry_research_target / (all_industry_industry_target + 0.0001)
, all_industry_research_target, all_industry_industry_target)
% (all_industry_research_target / (all_industry_industry_target + 0.0001),
all_industry_research_target, all_industry_industry_target)
print "Max Research assignments would result in target RP/PP ratio of %.2f ( %.1f / %.1f )" \
% (all_research_research_target / (all_research_industry_target + 0.0001)
, all_research_research_target, all_research_industry_target)
% (all_research_research_target / (all_research_industry_target + 0.0001),
all_research_research_target, all_research_industry_target)
print "-----------------------------------"
print "Final Ratio Target (turn %4d) RP/PP : %.2f ( %.1f / %.1f ) after %d Focus changes" \
% (fo.currentTurn(), current_research_target / (current_industry_target + 0.0001)
, current_research_target, current_industry_target, total_changed)
% (fo.currentTurn(), current_research_target / (current_industry_target + 0.0001),
current_research_target, current_industry_target, total_changed)

def print_table(self, priorityRatio):
"""Prints a table of all of the captured sections of assignments"""
Expand Down Expand Up @@ -276,7 +276,7 @@ def print_resources_priority():
# what is the focus of available resource centers?
print
warnings = {}
#TODO combine this with previous table to reduce report duplication?
# TODO combine this with previous table to reduce report duplication?
print "Planet Resources Foci:"
for planetID in empirePlanetIDs:
planet = universe.getPlanet(planetID)
Expand Down Expand Up @@ -427,8 +427,8 @@ def use_planet_growth_specials(focus_manager):

def use_planet_production_and_research_specials(focus_manager):
"""Use production and research specials as appropriate. Remove planets from list of candidates."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Please be more specific in the docstring.

What is happening in the function? Does it set any foci? Does it only calculate them? How does it "use" specials.

#TODO remove reliance on rules knowledge. Just scan for specials with production
#and research bonuses and use what you find. Perhaps maintain a list
# TODO remove reliance on rules knowledge. Just scan for specials with production
# and research bonuses and use what you find. Perhaps maintain a list
# of know types of specials
universe = fo.getUniverse()
already_have_comp_moon = False
Expand Down Expand Up @@ -481,7 +481,7 @@ def set_planet_protection_foci(focus_manager):

def set_planet_happiness_foci(focus_manager):
"""Assess and set planet focus to preferred focus depending on happiness"""
#TODO Assess need to set planet to preferred focus to improve happiness
# TODO Assess need to set planet to preferred focus to improve happiness


Copy link
Contributor

Choose a reason for hiding this comment

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

function seems missing a pass statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

pass required in no other code is present. It just like close bracket for indent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do comments actually count as well? Cause there sure isn't any actual code within the function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Comments does not count. Function have docstring. It is part of function declaration and can be accessed in runtime via __doc__

def set_planet_industry_and_research_foci(focus_manager, priorityRatio):
Expand All @@ -496,10 +496,10 @@ def set_planet_industry_and_research_foci(focus_manager, priorityRatio):
curTargetRP = 0.001
resource_timer.start("Loop") # loop
has_force = tech_is_complete("CON_FRC_ENRG_STRC")
#cumulative all industry focus
# cumulative all industry focus
ctPP0, ctRP0 = 0, 0
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of comment, use self-explaining variable names.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is copy paste of old code. But since it is local variables it can be changed here.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's what you get when spending 2 minutes to check a PR due to lack of time... Anyway, I guess not strictly needed to change it within this PR then, his call.

Copy link
Contributor

Choose a reason for hiding this comment

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

I plan to cleanup variable names and formatting myself. It is quite easy with IDE refactor tools and have less chances to got mistake.


#Handle presets which only have possible output for preset focus
# Handle presets which only have possible output for preset focus
for pid, pinfo in focus_manager.baked_planet_info.items():
nPP, nRP = pinfo.possible_output[pinfo.future_focus]
curTargetPP += nPP
Expand All @@ -515,7 +515,7 @@ def set_planet_industry_and_research_foci(focus_manager, priorityRatio):
if RFocus not in pinfo.planet.availableFoci:
focus_manager.bake_future_focus(pid, pinfo.current_focus, False)

#smallest possible ratio of research to industry with an all industry focus
# smallest possible ratio of research to industry with an all industry focus
maxi_ratio = ctRP0 / max(0.01, ctPP0)

for adj_round in [2, 3, 4]:
Expand Down Expand Up @@ -587,7 +587,7 @@ def set_planet_industry_and_research_foci(focus_manager, priorityRatio):
curTargetRP += (RR - IR)
curTargetPP -= (II - RI)

#Any planet in the ratios list and still raw is set to industry
# Any planet in the ratios list and still raw is set to industry
for ratio, pid, pinfo in ratios:
if pid in focus_manager.raw_planet_info:
focus_manager.bake_future_focus(pid, IFocus, False)
Expand Down