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

Add game rule to manage diplomatic interaction between players #2543

Merged
merged 1 commit into from Sep 15, 2019

Conversation

@o01eg
Copy link
Contributor

o01eg commented Sep 3, 2019

Uses set of allowed values to be extended in the future like in FreeCiv:

freeciv-d

@o01eg o01eg requested a review from geoffthemedio Sep 3, 2019
@o01eg o01eg self-assigned this Sep 3, 2019
@o01eg o01eg changed the title Add game rule to manage diplomatic interaction between players. Add game rule to manage diplomatic interaction between players Sep 3, 2019
Empire/EmpireManager.cpp Outdated Show resolved Hide resolved
"MULTIPLAYER", "ALLOWED_FOR_ALL", false,
DiscreteValidator<std::string>(std::set<std::string>({
"RULE_DIPLOMACY_ALLOWED_FOR_ALL",
"RULE_DIPLOMACY_FORBIDDEN_FOR_ALL"

This comment has been minimized.

Copy link
@geoffthemedio

geoffthemedio Sep 4, 2019

Member

Might also make sense to wrap the literals here... not sure what standard practice for this would be as I don't think there are many cases of DiscreteValidator with strings...

This comment has been minimized.

Copy link
@o01eg

o01eg Sep 4, 2019

Author Contributor

In the CombatEvents.cpp nothing is wrapped.

This comment has been minimized.

Copy link
@geoffthemedio

geoffthemedio Sep 7, 2019

Member

Those aren't using DiscreteValidator though.

Looking over code a bit, it looks like in (almost?) all cases, calls to OptionsDB::Add wrap their string-literal parameters for option name in UserStringNop, but in no case I saw does a call to GameRules::Add do so. It probably should in all cases, though it doesn't need to be done so for this pull request specifically.

Empire/EmpireManager.cpp Outdated Show resolved Hide resolved
@o01eg o01eg force-pushed the o01eg:game-rule-diplomatic branch from a1c4946 to 415b433 Sep 4, 2019
@geoffthemedio

This comment has been minimized.

Copy link
Member

geoffthemedio commented Sep 7, 2019

Some UI issues...
Diplomacy_Rule_Setup_Layout

@o01eg

This comment has been minimized.

Copy link
Contributor Author

o01eg commented Sep 7, 2019

@geoffthemedio Test rule for discrete validator looks alike.

@geoffthemedio

This comment has been minimized.

Copy link
Member

geoffthemedio commented Sep 7, 2019

Test rule doesnt have the same text overflow issues for me.

@o01eg

This comment has been minimized.

Copy link
Contributor Author

o01eg commented Sep 8, 2019

It looks like combobox issue. You could try to add characters to en.txt to reproduce it.

@geoffthemedio

This comment has been minimized.

Copy link
Member

geoffthemedio commented Sep 8, 2019

The issue is that the box is not wide enough for your text to fit, so one or both need to change size. Can you do that, or do you need / want me to?

@o01eg

This comment has been minimized.

Copy link
Contributor Author

o01eg commented Sep 8, 2019

@geoffthemedio I think it's a dropdownlist issue: #2545

@geoffthemedio

This comment has been minimized.

Copy link
Member

geoffthemedio commented Sep 14, 2019

DropLists, including StringRuleWidget be made to now allow row contents to render outside them, but it doesn't really help much with this issue...
diplo_drop
I think your text is just too long for this space... even if the droplist is made a bit wider, more than one word is difficult to fit and all the text starts with the same word. Much wider and the description won't fit well...

Edit: I made the droplists (and the whole setup window and rules panel) wider, so the abbreviated text suggested below can fit.

Uses set of allowed values to be extended in the future.
@o01eg o01eg force-pushed the o01eg:game-rule-diplomatic branch from 415b433 to 06583ca Sep 14, 2019
@geoffthemedio geoffthemedio merged commit c7b5a6a into freeorion:master Sep 15, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@o01eg o01eg deleted the o01eg:game-rule-diplomatic branch Sep 15, 2019
@Vezzra Vezzra added this to the v0.4.9 milestone Sep 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.