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

Fix ResourceSupplyConnectedByEmpire condition #2217

Conversation

Projects
2 participants
@Dilvish-fo
Copy link
Member

commented Jul 15, 2018

  • so that it handles the fact that blockaded planets are still resource-connected to themselves

Some potential for confusion remains, in that:

  • the related matter of blockaded planets having their production points available to them is
    currently handled in ResourcePool::Update() solely with regards to the empire's
    m_resource_pools[RE_INDUSTRY].m_connected_object_groups_resource_output
    without making corresponding changes to
    m_resource_pools[RE_INDUSTRY].m_connected_system_groups
    (nor to GetSupplyManager().ResourceSupplyGroups(m_id))
    and perhaps m_connected_system_groups should be renamed to be m_connected_object_groups and
    get a similar update

Fixes #2215

Fix ResourceSupplyConnectedByEmpire condition
- so that it handles the fact that blockaded planets are still resource-connected to themselves

Some potential for confusion remains, in that:
- the related matter of blockaded planets having their production points available to them is
  currently handled in ResourcePool::Update() solely with regards to the empire's
      m_resource_pools[RE_INDUSTRY].m_connected_object_groups_resource_output
  without making corresponding changes to
      m_resource_pools[RE_INDUSTRY].m_connected_system_groups
  (nor to GetSupplyManager().ResourceSupplyGroups(m_id))
  and perhaps m_connected_system_groups should be renamed to be m_connected_object_groups and
    get a similar update
@agrrr3

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2018

Hm. Without digging deeper ... shouldnt the fix go into SupplyManager::Update ?
After digging a bit deeper - SupplyManager currently only talks about systems, so it shouldnt concern individual planets.
I feel there should be a more central place to handle this case but my cpp is not deep enough to suggest a reasonably efficient implementation.

@Dilvish-fo Dilvish-fo merged commit a79dbd7 into freeorion:master Jul 23, 2018

2 checks passed

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

@Dilvish-fo Dilvish-fo deleted the Dilvish-fo:fix_resource_supply_connection_condition branch Jul 23, 2018

@Dilvish-fo Dilvish-fo moved this from Proposed to Accepted in 0.4.8 Release Jul 23, 2018

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.