Martial faction points #79
Conversation
valdisz
force-pushed
the
flexible-faction-points
branch
from
e80411e to
7ee907c
Compare
| "Trade", | ||
| "Magic" | ||
| }; | ||
| const std::string F_WAR = "War"; |
There was a problem hiding this comment.
why do we changing this to std string?
There was a problem hiding this comment.
So that there is no unexpected behavior in using strings with STD containers as I don't trust AString. std::string is better in all aspects and there are no problems to convert it into AString.
std::string is available since 1998. It existed already when Atlantis code was written. AString should not exist then, and it is wrong to use it now.
| } | ||
|
|
||
| bool Faction::IsActivityRecorded(ARegion *region, FactionActivity type) { | ||
| auto regionActivity = this->activity[region]; |
There was a problem hiding this comment.
was it pushed "as it is" or changes to auto?
There was a problem hiding this comment.
IsActivityRecorded is a new function.
There was a problem hiding this comment.
auto does not mean that variable will have a dynamic type or something. It just tells that compiler should infer type during compilation. It is the same as var in C# or Java.
No need to write long type names if the compiler knows them.
| F_MAGIC, | ||
| NFACTYPES | ||
| }; | ||
| extern const std::string F_WAR; |
There was a problem hiding this comment.
if we change FWAR to enum, can we keep the same AString?
There was a problem hiding this comment.
I would be more afraid of using AString instead of std::string.
The enum will make code more complicated because it will need those constants anyway (to be used in parsing) and it will need mapping.
| @@ -58,6 +58,11 @@ static int ag[] = { 0, 1, 2, 4, 6, 10 }; | |||
| int *allowedTacticians = ag; | |||
| int allowedTacticiansSize = sizeof(ag) / sizeof(ag[0]); | |||
|
|
|||
| // allowed Martial activity | |||
| static int ma[] = { 0, 10, 25, 40, 60, 90 }; | |||
There was a problem hiding this comment.
why do we need this here if it is off?
There was a problem hiding this comment.
Otherwise, code won't compile and all other rulesets will be broken.
|
And look, 7 mages for Fracas ;)
…Sent from my iPhone
On 22 Nov 2021, at 18:35, Valdis Zobēla ***@***.***> wrote:
@valdisz commented on this pull request.
In faction.h:
> @@ -52,12 +56,10 @@ enum {
NATTITUDES
};
-enum {
- F_WAR,
- F_TRADE,
- F_MAGIC,
- NFACTYPES
-};
+extern const std::string F_WAR;
I would be more afraid of using AString instead of std::string.
The enum will make code more complicated because it will need those constants anyway (to be used in parsing) and it will need mapping.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
Yes, was working fine. Need to admit that this was not quite big test game. |
This PR merges War and Trade points into Martial points. Activity tracking is made more flexible in the code.
Faction points are defined during runtime and are using vector.
#71