Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Feature #738016

closed

Stack bribe

Added by Zoltán Žarkov over 6 years ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Category:
General
Sprint/Milestone:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

I am backporting stack bribe from warclient repo. In new option, spies will be able to bribe all units in the target tile.


Files

Actions #1

Updated by Sveinung Kvilhaugsvik over 6 years ago

I suggest that you start with a copy of the regular "Bribe Unit" action, rename it and change its target kind to ATK_UNITS. (Do a search for ACTION_SPY_BRIBE_UNIT to find starting points for what should be copied.) Then do other modifications to the copy to make it match the warclient.

Actions #2

Updated by Zoltán Žarkov over 4 years ago

  • Sprint/Milestone set to 3.1.0
Actions #3

Updated by Zoltán Žarkov over 4 years ago

  • Blocks Task #673656: S3_1 datafile format freeze (d3f) added
Actions #5

Updated by Zoltán Žarkov over 4 years ago

  • Priority changed from Low to Normal

Putting some text in so it shows up in activity.

Actions #6

Updated by Zoltán Žarkov over 4 years ago

- Fixed client/include/dialogs_g.h

Actions #7

Updated by Marko Lindqvist over 4 years ago

Will sveinung look at this as it's actions related?

Actions #8

Updated by Sveinung Kvilhaugsvik over 4 years ago

Marko Lindqvist wrote:

Will sveinung look at this as it's actions related?

Yes

Actions #9

Updated by Zoltán Žarkov over 4 years ago

- Fixed indentation in server/unithand.c

Actions #10

Updated by Alexandro Ignatiev over 4 years ago

May we just skip units that we can't bribe (especially our own ones, especially that treacherous diplomat himself) if we can bribe other units? This contradicts is_action_enabled_unit_on_units_full but maybe that's the reason it should be a unit-on-tile action? Or just don't require that target units are foreign, put "any foreign units on the tile" as a hard requirement and catch the cases when they are all unbribable with a sanity check? Or do something to ATK_UNITS in general?

Actions #11

Updated by Zoltán Žarkov over 4 years ago

I tried to keep it as simple as possible like capture units to enable the normal sorts of ruleset options. No objection to allowing more complex things like that later, but I was trying to avoid weird edge cases.

Actions #12

Updated by Sveinung Kvilhaugsvik over 4 years ago

  • Category set to General
  • Status changed from New to In Progress

Zoltán Žarkov wrote:

I tried to keep it as simple as possible like capture units to enable the normal sorts of ruleset options.

Good choice. An initial patch should, especially when I'm supposed to review it, be simple. It is easier to find time to review a small patch that does one thing than to find time to review a large patch or a patch that does more than one thing.

Below feed back may apply to more than one location in your patch. If an issue exists in one client check the other clients too.

You have some issues with extra white space at the end of lines. gitt diff log -p will mark them for you.

Please add documentation to doc/README.actions

Where checking action result: Please try to avoid assuming that action id equals action result. Cleaning up those are the main thing holding back proper generalized actions. You can use action_has_result() instead. Other than that I must say you did a great job of integrating your changes with the action sub system.

const bool tile_targeted = paction->id == ACTION_SPY_BRIBE_UNITS;

Please check action target type directly. See action_get_target_kind()

/* TRANS: %s is pre-pluralised "Treasury contains %d gold." */

Is this translation comment useless now or is it still extracted? I think it needs to be directly above the string.

Why not insert target kind text ("unit stacks") as a string rather than doing the if statement?

- && NULL != game_unit_by_number(pBribe_Dlg->target_id)) {
+ && (pBribe_Dlg->act_id == ACTION_SPY_BRIBE_UNITS
+ || game_unit_by_number(pBribe_Dlg->target_id))) {

Maybe check that the target tile is valid (like the check if the unit target is valid for regular bribe) in cases like this?

case ACTION_SPY_BRIBE_UNIT:
+ case ACTION_SPY_BRIBE_UNITS:
/* All uncertainty comes from potential diplomatic battles. */

I think you are correct and that potential hidden (by transport, invisibility, etc) units are detected as uncertainty. Have you tested it?

Actions #13

Updated by Sveinung Kvilhaugsvik over 4 years ago

Corner case (that could be handled in a follow up patch): it is impossible to bribe a unit capable of diplomatic battle defense with this action.

Actions #14

Updated by Sveinung Kvilhaugsvik over 4 years ago

Heads up: if Feature #873599 and Feature #873605 lands before this patch you will have to set the unit actor post action move info in hard_code_actions() rather than in utype_is_moved_to_tgt_by_action(). (Just replace the call to action_new() with a call to unit_action_new() and set the new parameter)

Actions #15

Updated by Sveinung Kvilhaugsvik over 4 years ago

Feature #873599 and Feature #873605 just landed.

Actions #16

Updated by Marko Lindqvist almost 3 years ago

  • Status changed from In Progress to New
  • Assignee deleted (Zoltán Žarkov)

Are we going to do something like this to 3.1? This is still listed as a S3_1 d3f blocker.

Actions #17

Updated by Marko Lindqvist over 2 years ago

  • Sprint/Milestone changed from 3.1.0 to 3.2.0

Marko Lindqvist wrote:

Are we going to do something like this to 3.1?

From the silence of additional four months I'm assuming not.

Actions #18

Updated by Marko Lindqvist over 2 years ago

  • Blocks Task #939772: S3_2 datafile format freeze (d3f) added
Actions #19

Updated by Marko Lindqvist over 2 years ago

  • Blocks deleted (Task #673656: S3_1 datafile format freeze (d3f))
Actions #20

Updated by Marko Lindqvist 3 months ago

  • Status changed from New to Closed
  • Assignee set to Marko Lindqvist
  • Sprint/Milestone changed from 3.2.0 to 3.3.0
Actions #21

Updated by Marko Lindqvist 3 months ago

  • Blocks deleted (Task #939772: S3_2 datafile format freeze (d3f))

Also available in: Atom PDF