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 #6233: Improve lock pricing and infrastructure counting, to achieve better consistency #6933

Closed

Conversation

SamuXarick
Copy link
Contributor

When building lock:

  • don't add the cost for clearing river
  • if not building on a river, simulate canal build price and add it to the landscape clearing cost
  • if not building on a river, simulate canal maintenance count

When removing lock:

  • if it was not built on a river, simulate the cost for clearing a canal tile and add it to the cost for clearing the lock itself
  • if it was not built on a river, subtract canal maintenance count

When transfering ownership:

  • old owner: subtract canal maintenance count if it was not built on a river
  • new owner: simulate canal maintenance count if it was not built on a river

After loading a savegame:

  • simulate canal maintenance count if it was not built on a river

@SamuXarick SamuXarick force-pushed the lock-pricing-adjustments branch 2 times, most recently from 9e771c9 to c40417a Compare October 6, 2018 02:42
@frosch123
Copy link
Member

This seems to do multiple things:

  • Change how locks are counted as infrastructure.
  • Change construction checks of locks by not checking the clear-result in case of IsWaterTile.
  • Change cost of building/clearing locks.

I do not agree with the new counting of infrastructure. The old one made more sense to me.

I am unsure whether the clear-check can fail for IsWaterTile.

I do not quite understand the intention of the cost change. I guess there are different ideas of what is part of a lock.
However, the diff seems to make construction of locks on rivers cheaper, which does not make much sense to me. I would expect the reverse.

@nielsmh
Copy link
Contributor

nielsmh commented Oct 7, 2018

Conceptually, a river should not be able to support a lock by itself. Building a lock on a river should entail effectively converting the river to a canal on those tiles. So, thumbs down from me.

@SamuXarick
Copy link
Contributor Author

For consistency!

When building a canal on a river, it doesn't include the cost for clearing river. When building a ship depot on a river, it doesn't include the cost for clearing river. Even the north and south parts of a lock don't include the cost for clearing river. The watered part of a dock doesn't include the cost of clearing river either. So it would also make sense to make the middle tile of a lock to not include the cost of clearing the river.

As for simulating canal price and canal count… This canal tile isn't actually built or demolished, but its associated costs are simulated, much in the sense to mimic what happens on the upper and lower tiles of a lock. Objective here is to achieve pricing consistency, but mostly to fix a flaw that can be exposed using BaseCostsMod 5.0 as I reported in #6233.

If the value for building a canal was instead so blatantly much higher than the cost for building a lock itself, the flaw would become exposed: building several locks in a row would be cheaper than building a single contiguous canal line.

@TrueBrain
Copy link
Member

We recently switched from Jenkins as CI to Azure Pipelines as CI. This means you need to rebase before this Pull Request will pass its checks. Sorry for the troubles!

…o achieve better consistency

When building lock:
- don't add the cost for clearing river
- if not building on a river, simulate canal build price and add it to the landscape clearing cost
- if not building on a river, simulate canal maintenance count

When removing lock:
- if it was not built on a river, simulate the cost for clearing a canal tile and add it to the cost for clearing the lock itself
- if it was not built on a river, subtract canal maintenance count

When transfering ownership:
- old owner: subtract canal maintenance count if it was not built on a river
- new owner: simulate canal maintenance count if it was not built on a river

After loading a savegame:
- simulate canal maintenance count if it was not built on a river
@LordAro LordAro added needs triage This issue needs further investigation before it becomes actionable needs review labels Jan 20, 2019
@andythenorth
Copy link
Contributor

Per Frosch's review, this won't be accepted as a change, both conceptually, and in the way it's implemented. Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review needs triage This issue needs further investigation before it becomes actionable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants