Menu

#614 Undo maritime trade.

None
open
nobody
None
5
2013-09-08
2011-08-29
No

Allows undo maritime trade. I dug deep into the state machine for this, the only way to learn it...

The one thing I missed was to place the bank trade back into the trade quote, but for now, just refresh it by deselecting and reselecting a resource check box.

Discussion

  • Micah Bunting

    Micah Bunting - 2011-08-31

    While I was using this patch, the server crashed on me during a 4:1 trade, I have no log, I need to troubleshoot it.

     
  • Micah Bunting

    Micah Bunting - 2011-09-09

    Fixed the segfault bug and added V0_13 in ClientVersionType to prevent an error message with older clients.

     
  • Micah Bunting

    Micah Bunting - 2011-09-10

    allows undo maritime trade. version 3

     
  • Roland Clobus

    Roland Clobus - 2013-06-12

    I've updated the patch to match the current code (svn revision 1946)

    I think this feature is nice to have in the Pioneers 15 release.
    I would propose to change the following (assign the issue to me to let me make them):

    • The undo action shows a '-1' for a resource, which appears strange to me.
    • Use 'undo-maritime-trade %d supply %r receive %r' instead of 'undo-trade' (to match the 'maritime-trade')
    • Instead of adding a gint[NO_RESOURCE] add 3 members to buildrec: maritime_ratio, maritime_supply, maritime_receive.
    • Similarly, in the reconnect: use 'maritime-trade %d supply %r receive %r' instead of 'tradeinfo: %R'
    • The view must show the maritime trade again (which can be based on the data in 'undo-maritime-trade')
    • buildrec_is_trade_only is unclear to me: buildrec_is_empty_or_trade_only or buildrec_something_built_or_bought
    • I did not test yet with a Pioneers 14 client. Pre-15 clients should have at least some notification that the trade was undone.
     
  • Micah Bunting

    Micah Bunting - 2013-06-12
    • assigned_to: Roland Clobus
    • Group: -->
     
  • Micah Bunting

    Micah Bunting - 2013-06-12

    Go ahead and make the changes. I was thinking about addressing this patch after my current thread, but what will be awhile. Just interesting how time works. I had more time to work on things in the past, while you where busy. Now it seems to be the other way.

     
  • Roland Clobus

    Roland Clobus - 2013-09-08
    • assigned_to: Roland Clobus --> nobody
     
  • Roland Clobus

    Roland Clobus - 2013-09-08

    As mailed to pio-develop on 2013-09-02

    MB For patch 614 undo maritime trade, you said “I think this feature is
    MB nice to have in the Pioneers 15 release.” Are you going to add this
    MB patch in the release?

    RC I was working on it. But I've found that the undo system has some subtle
    RC bugs, unexpected behaviours/code paths, doesn't follow the master/slave
    RC model and then I got side-tracked.

    Therefore I unassign the patch. I still think this would a good feature, but it will not make it for V15.

     

Log in to post a comment.