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

CLASSIC caravan revenue style uses a weird distance #882

Closed
lmoureaux opened this issue Feb 7, 2022 · 0 comments · Fixed by #2094
Closed

CLASSIC caravan revenue style uses a weird distance #882

lmoureaux opened this issue Feb 7, 2022 · 0 comments · Fixed by #2094
Labels
bug Something isn't working oddity Something that is strange or doesn't make sense

Comments

@lmoureaux
Copy link
Contributor

Describe the bug

With caravan_bonus_style CLASSIC, the revenue is calculated using map_distance instead of real_map_distance. There is even a comment saying Should this be real_map_distance?. map_distance corresponds to the Manhattan distance and real_map_distance to the Euclidean one.

As a consequence of this, moving a caravan diagonally grants a bonus roughly 2x the bonus of a caravan moving along x or y (for the same number of moves).

To Reproduce
Steps to reproduce the behavior:

  1. Read the code (common/traderoutes.cpp)

Expected behavior

I propose to change this to real_map_distance instead. A correction factor could be introduced to maintain the balance of existing rulesets (on average).

Platform and version (please complete the following information):

  • OS: any
  • Freeciv21 version: master
  • Ruleset/Longturn game (if applicable): any of the shipped rulesets with caravans that can Enter marketplace

Additional context

#880
@lmoureaux lmoureaux added bug Something isn't working oddity Something that is strange or doesn't make sense labels Feb 7, 2022
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this issue Dec 27, 2023
@lmoureaux
Use the Euclidian distance for CLASSIC trade route revenue
a27a9c9 
The CLASSIC trade route revenue style was using map_distance() which is the
Manhattan distance. Using Euclidian makes much more sense since this is how
units move.

Closes longturn#882.
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this issue Dec 27, 2023
@lmoureaux
Switch to the Euclidian distance for trademindist
9dddcdc 
See longturn#882.
@lmoureaux lmoureaux mentioned this issue Dec 27, 2023
Merged
lmoureaux added a commit that referenced this issue Dec 27, 2023
@lmoureaux
Use the Euclidian distance for CLASSIC trade route revenue
9892203 
The CLASSIC trade route revenue style was using map_distance() which is the
Manhattan distance. Using Euclidian makes much more sense since this is how
units move.

Closes #882.
lmoureaux added a commit that referenced this issue Dec 27, 2023
@lmoureaux
Switch to the Euclidian distance for trademindist
cdf3ea3 
See #882.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working oddity Something that is strange or doesn't make sense
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove Manhattan distances from trade routes lmoureaux/freeciv21
1 participant
@lmoureaux