Attachments (2)

ptol-teambonus.patch (5.0 KB) - added by mimo 3 years ago.
globalauras.diff (15.6 KB) - added by fatherbushido 3 years ago.

Download all attachments as: .zip

Change History (40)

comment:1 Changed 3 years ago by stanislas69

Description: modified (diff)

comment:2 Changed 3 years ago by elexis

Keywords: simple added

comment:3 Changed 3 years ago by mimo

Keywords: simple removed

Auras are based on a rangeManager query to find the entities on range, but the range manager only works for entities with a position and inWorld. So currently most team bonuses (using global auras) are broken as the player entity has no position: auras are correctly applied to templates, but not to entities. A possible solution would be that CCmpRangeManager still performs the query (using 0-0 as position) for such special entities.

There are also some side-effects for this: for example hero auras are not applied to entities created while the hero is garrisoned, it is only when we ungarrison it that the aura is applied. Here, when querying a garrisoned entity, we could take (recursively as a garrisonHolder may be garrisoned) the position of its garrisonHolder.

Finally, once team bonuses works, they should be disabled when the allied at the root of the aura is defeated.

comment:4 Changed 3 years ago by mimo

Keywords: rfc patch added
Milestone: BacklogAlpha 21
Summary: Design Docs : Team Bonuses[PATCH] Team Bonuses (from design doc)

Some global auras, affecting only the templates and potentially the player entities, make a useless rangeQuery (as player entities have no position). Here is a patch to avoid this rangeQuery. As an example of such use, the patch also implements the ptol team bonus. (As the ResourceTrickle? component was used, it also contains some optimization for it, recomputing the rates only when modified and not on each iteration). Finally, there is a TODO in the patch which is here for discussion, about defining a new "player" type for such auras, instead of using the "global" one.

Changed 3 years ago by mimo

Attachment: ptol-teambonus.patch added

comment:5 Changed 3 years ago by sanderd17

Something like this would get rid of the range query, though it still needs extra code for when the receiver is the player entity directly: http://pastebin.com/9460Khhn

It's also completely untested and most likely broken. It serves mostly as an example to start from (f.e. it minimises the global message listeners by letting the AuraManager? listen to it (most aura entities don't have a global aura so wouldn't need the listener anyway)).

Last edited 3 years ago by sanderd17 (previous) (diff)

comment:6 Changed 3 years ago by fatherbushido

Description: modified (diff)

comment:7 Changed 3 years ago by fatherbushido

I try to make it clean in my head.

Problems:

  1. when the source is the Player entity : entities are not affected (range query fails as the source has no position)
  2. when the source is garrisoned, the aura is not immediately applied : range query fails as the source has no position
  3. when the target is a Player entity : there range query is useless
  4. when the aura is global, there are useless range queries
  5. team bonuses aren't remove when player is defeated

I can perhaps add

  1. The source of an aura is not affected by a range aura (but perhaps it is expected like that).

Solutions:

  1. possible solution suggested in comment1. Perhaps we can use cmpRangeManager.GetEntitiesByPlayer?() instead ? (and a special (from)player aura type in this case ?)
  2. same
  3. adressed in mimo's patch. Indeed in this case a (to)player aura type can be good
  4. adressed in sanderd17 's patch
  5. we must listen to PlayerDefeated? message in aura component and...
Last edited 3 years ago by fatherbushido (previous) (diff)

comment:8 Changed 3 years ago by fatherbushido

Milestone: Alpha 21Alpha 22

comment:9 Changed 3 years ago by javiergodas

I'm trying to do Socii bonus now

comment:10 Changed 3 years ago by fatherbushido

Description: modified (diff)

comment:11 Changed 3 years ago by fatherbushido

Description: modified (diff)

comment:12 Changed 3 years ago by fatherbushido

Description: modified (diff)
Summary: [PATCH] Team Bonuses (from design doc)[PATCH] Team Bonuses (for all civs)

comment:13 Changed 3 years ago by scythetwirler

Keywords: beta added

comment:14 Changed 3 years ago by fatherbushido

Description: modified (diff)

comment:15 Changed 3 years ago by fatherbushido

Description: modified (diff)

comment:16 Changed 3 years ago by fatherbushido

Description: modified (diff)

comment:17 Changed 3 years ago by fatherbushido

@mimo: consider ptol-teambonus.patch​ as tested and reviewed by me. (both the auras and the resourceTrickle part). (Perhaps 1 instead of 1.5 is better for the food rate and in the comment 'The player entity may also be a ResourceTrickle? component' -> perhaps it's have). I (or someone else) will then investigate and merge the sanderd17 patch. It will remain the player defeated issue.

Last edited 3 years ago by fatherbushido (previous) (diff)

comment:18 Changed 3 years ago by mimo

In 18920:

implement ptol civ bonus and optimization of ResourceTrickle? component, refs #4082

comment:19 Changed 3 years ago by mimo

Thanks fatherbushido for the review, I've switched the rate to 1.

comment:20 Changed 3 years ago by elexis

Keywords: rfc removed

comment:21 Changed 3 years ago by elexis

As seen in #3934, Trickle could just use AddResources instead of a loop over AddResource

comment:22 Changed 3 years ago by fatherbushido

I have checked and brought corrections to the (precious) pastebin draft given by sanderd17. I will test it even more deeply, review it again, and try to write more complete tests.

EDIT:

It looks ready to go. I wrote some more tests (wich would fail without the current code). They could be written in an even clear way. I test in game. I add the mauryan team bonus as in design docs. For the team bonus aura name, I used what I found in Wikipedia: In his edicts, Ashoka expresses support for all the major religions of his time. I hesitated to use Proselytism as in Delenda Est mod.

At last, it will remain the cleaning TODO when the target is a player entity. But it will be done in a separate patch.

Version 2, edited 3 years ago by fatherbushido (previous) (next) (diff)

Changed 3 years ago by fatherbushido

Attachment: globalauras.diff added

comment:23 Changed 3 years ago by fatherbushido

Keywords: review added

comment:24 Changed 3 years ago by fatherbushido

Keywords: review removed

refs r19092 r19093

comment:25 Changed 3 years ago by fatherbushido

Description: modified (diff)

comment:26 Changed 3 years ago by fatherbushido

Description: modified (diff)

It remains the "player" type TODO (for Player target).

For the remaining team bonus: brit, gaul are easy, cart, pers are easy (but nead to make consistency / cleaning with yet existing trading bonus in templates / tech), rome, mace needs some work, spart needs work or redesign.

Last edited 3 years ago by fatherbushido (previous) (diff)

comment:27 Changed 3 years ago by fatherbushido

Description: modified (diff)

refs r19102

comment:28 Changed 3 years ago by fatherbushido

refs r19117

comment:29 Changed 3 years ago by fatherbushido

http://trac.wildfiregames.com/wiki/Civ%3A_Britons#TEAMBONUS

http://trac.wildfiregames.com/wiki/Civ%3A_Gauls#TEAMBONUS

http://trac.wildfiregames.com/wiki/Civ%3A_Macedonians#TEAMBONUS

http://trac.wildfiregames.com/wiki/Civ%3A_Romans_Republican#TEAMBONUS

http://trac.wildfiregames.com/wiki/Civ%3A_Spartans#TEAMBONUS

Well there are 5 team bonus remaining. For some of them, I guess we can think something else. For others we can keep the same spirit but perhaps change the effect. And at last, if some are remaining perhaps find a placeholder.

brit, gaul: "Druides". Any other suggestion or effect? mace: "Standardized currency". Any effect idea? I thought something related to barter? (we should implement modification of a constant). I thought also to "League of Corinth" https://en.wikipedia.org/wiki/League_of_Corinth rome: "socci" seems hard to do with territory. Any idea? spart: "Peloponean" League. That sounds good but I think to find a modification abstracting the fact that spartan protect their allies (in the scope of that league).

Last edited 2 years ago by fatherbushido (previous) (diff)

comment:31 Changed 3 years ago by fatherbushido

Yes Imarok! Do it ;-) Last time I wanted to do that, I gave up. The hybrid state of those files is a bit confusing. If you have a clear input about that please share! Special techs and many things in that should be updated too.

comment:32 Changed 2 years ago by fatherbushido

Description: modified (diff)

comment:33 Changed 2 years ago by fatherbushido

comment:34 Changed 2 years ago by fatherbushido

In 19591:

Team bonuses for Britons, Gauls and Romans. Reviewed by elexis. Refs #4082.
Differential Revision: https://code.wildfiregames.com/D502

comment:35 Changed 2 years ago by fatherbushido

Description: modified (diff)

comment:36 Changed 2 years ago by fatherbushido

In 19616:

Add Macedonian team bonus. Refs #4082. Reviewed by mimo.
Differential Revision: ​https://code.wildfiregames.com/D480

comment:37 Changed 2 years ago by fatherbushido

Keywords: beta removed
Resolution: fixed
Status: newclosed

comment:38 Changed 2 years ago by Imarok

In 19673:

Update teamboni in civ.jsons

Reviewed by: fatherbushido
refs #4082
Differential Revision: https://code.wildfiregames.com/D460

Note: See TracTickets for help on using tickets.