bugFreeciv - Bugs: bug #22243, City with more than max trade...

 
 
Show feedback again

bug #22243: City with more than max trade routes can't change them

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sat 28 Jun 2014 09:16:11 PM UTC  
 
Category: generalSeverity: 2 - Minor
Priority: 5 - NormalStatus: Fixed
Assigned to: pepeto <pepeto>Open/Closed: Closed
Release: Operating System: Any
Planned Release: 2.5.0, 2.6.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Tue 25 Nov 2014 09:14:26 AM UTC, SVN revision 27129:

Allow a city with more than maximal trade route number to establish a new trade
route if the trade income of the new trade route is greater than the sum of
the weakest trade routes to cancel.

Reported by Jacob Nevins

See gna bug #22243

(Browse SVN revision 27129)

pepeto <pepeto>
Project MemberIn charge of this item.
Tue 25 Nov 2014 09:14:24 AM UTC, SVN revision 27128:

Allow a city with more than maximal trade route number to establish a new trade
route if the trade income of the new trade route is greater than the sum of
the weakest trade routes to cancel.

Reported by Jacob Nevins

See gna bug #22243

(Browse SVN revision 27128)

pepeto <pepeto>
Project MemberIn charge of this item.
Tue 11 Nov 2014 11:41:05 AM UTC, comment #3:

Updated against svn HEAD.

Additionally, make the string "The city of XXX already has YYY better trade routes!" only visible if "YYY > 0" and with plural form translations.

Now, I just need to make a savegame able to test the feature correctly...

(file #22867, file #22868)

pepeto <pepeto>
Project MemberIn charge of this item.
Sun 20 Jul 2014 07:00:31 PM UTC, comment #2:

Attached patch applies trunk and S2_5.

> (establish_trade_route() also checks ==max, but in that case
> assertions rather than graceful handling are appropriate.)


I have replaced there also by >=max, because the function potentially can be called by another part. However, I also added assertions to ensure there is still a free slot.

Additionally, I have removed the test in remove_smallest_trade_route() which looked incorrect (the trade value may be 0, even if a trade route is established).

In base_handle_unit_establish_trade(), removed the city info from pcity_out_of_home to pcity_out_of_dest and its reverse. I don't understand it, it looks like enabling cheating...

(file #21483)

pepeto <pepeto>
Project MemberIn charge of this item.
Sat 28 Jun 2014 09:38:02 PM UTC, comment #1:

Also, can_establish_trade_route() checks for ==max, so currently gives the wrong answer in the face of excess routes. It should be updated for whatever the conclusion of this is.

(establish_trade_route() also checks ==max, but in that case assertions rather than graceful handling are appropriate.)

Jacob Nevins <jtn>
Project Administrator
Sat 28 Jun 2014 09:16:11 PM UTC, original submission:

After the fix for bug #21141 (as modified by bug #21152), if a city ends up with more trade routes than the current maximum, I think the routes become frozen; it's impossible for the player to influence them by establishing new routes, and if they try they'll get a potentially inaccurate message "The city of Linz already has <current max> better trade routes!", even if the new route would have had better trade than any of the existing ones. (Not tested this.)

The reduction in max traderoutes might not be something they can easily reverse (wonder loss, tech loss, ...). Depending on the ruleset, players might be forced to resort to giving away their city and retaking it (causing routes to be cancelled) to regain control of its trade routes.
This may limit the practical use of the Max_Trade_Routes effects in rulesets to requirements likely to survive indefinitely.

I think a sensible solution would be to allow establishing a new route in this case iff the new one would yield more than the sum of the weakest N routes where N = excess+1, and have the new route cancel all of those routes, bringing the total within the current max.
(This will be slightly fiddly to implement since it involves sorting/ranking the routes.)

  • (If we're going to add the ability to sort the routes, I wonder about an alternative behaviour for excess traderoutes where the routes that would yield least in a given turn become inactive, yielding nothing. Establishing a new route in this case would have the same behaviour, bringing the number of routes down to the current max, treating the excess routes as having value 0.)

Alternatively, the minimal code change would be to disable the "N better trade routes" message, leaving just the bare infuriating "Sorry, your Caravan cannot establish a trade route here!" (but that doesn't improve gameplay, of course).

Jacob Nevins <jtn>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by pepeto (Updated the item)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 25 Nov 2014 09:14:36 AM UTCpepetoStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Tue 11 Nov 2014 11:41:05 AM UTCpepetoAttached File-=>Added trunk_allow_displace_trade_over_max2.patch, #22867
      Attached File-=>Added S2_5_allow_displace_trade_over_max2.patch, #22868
      Assigned toNone=>pepeto
    Sun 20 Jul 2014 07:00:31 PM UTCpepetoAttached File-=>Added allow_displace_trade_over_max.patch, #21483
      CategoryNone=>general
      StatusNone=>Ready For Test
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup