Page MenuHomeWildfire Games

Trade with neutral should be allowed in the gui
ClosedPublic

Authored by mimo on Jan 3 2017, 6:50 PM.

Details

Summary

We've always been supposed to be able to trade with neutrals, and the Trade component allows it, so this gui restriction should be removed

Test Plan

already tested

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mimo updated this revision to Diff 118.Jan 3 2017, 6:50 PM
mimo retitled this revision from to Trade with neutral should be allowed in the gui.
mimo updated this object.
mimo edited the test plan for this revision. (Show Details)
mimo set the repository for this revision to rP 0 A.D. Public Repository.
Vulcan added a subscriber: Vulcan.Jan 3 2017, 8:03 PM

Build is green

Updating workspaces.
Build (release)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
../../../source/lib/tex/tex_png.cpp: In member function ‘virtual Status TexCodecPng::encode(Tex*, DynArray*) const’:
../../../source/lib/tex/tex_png.cpp:309:9: warning: variable ‘ret’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
  Status ret = ERR::FAIL;
         ^
Build (debug)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/81/ for more details.

leper accepted this revision.Jan 4 2017, 12:13 AM
leper added a reviewer: leper.
This revision is now accepted and ready to land.Jan 4 2017, 12:13 AM
elexis added a subscriber: elexis.Jan 4 2017, 1:19 AM

We have to keep in mind that the alliance and neutral stance should be distinct and implement different use cases. Besides the ability to trade, only the shared ally vision and territory decay comes to my mind. For example if we had an option to limit the number of alliances per player to N, this would incentivize players to pick their allies more wisely. But you're likely right that it makese sense to allow trade amongst neutral players, it isn't unrealistic either.

This revision was automatically updated to reflect the committed changes.

In order to keep distinct the position of a player between neutral and allied, probably trade should be allowed between allies only, making clear the player's position.

elexis changed the visibility from "All Users" to "Public (No Login Required)".Jun 26 2017, 2:24 PM