Conversation
''mass upgrades in the summarized troop window bypassing restrictions and clicking "done" prevents closing the party menu, but the upgrades have already occurred?'' bug. changes summary may look a little cursed cuz i was using nuget.
This reverts commit bbb704f.
a fix to ''mass upgrades in the summarized troop window bypassing restrictions and clicking "done" prevents closing the party menu, but the upgrades have already occurred'', bug. ui costs update late, no idea why, not a big problem either. replaced DoneLogic n late checks, they run after PartyScreen queues ops racy, allowed upgrades on ctrl spam and miscounted costs. reserve in AddCommand, affordable applied, clamp TotalNumber, if 0 so it prevents ctrl chains and stale party ui totals(cant really)
fixed ui bugs and and improved readability. second time pushing it because something was wrong with git.
added enchantment ingredients for autoresolved battles added quality checks for runesmith quests fixed a bug in runelord quest fixed grain -> oathgold hint text fixed herdstone/portal drops fixed rune typo fixed revive skill added names for new templates so you dont have to restart your game when you accidently craft them didnt touch reread books bug
…OR_Core into testing-and-stuff
This reverts commit 75c14ce.
autores now gives enchantment ingredients miners guild troop donation oathgold gain fixed grain to oathgold hint mismatch fixed herdstone/portal rewards fixed rune typo fixed faith revive perk fixed runelord quest fixed new template ids added so you dont have to close the game after crafting them by accident
git didnt push them for some reason
|
|
||
| hint.SetTextVariable("WHEAT_COUNT", 30); | ||
| hint.SetTextVariable("OATH_GOLD_GAIN_WHEAT",SteelGain ); | ||
| hint.SetTextVariable("OATH_GOLD_GAIN_WHEAT", 30 / 2); |
There was a problem hiding this comment.
why did we change here to magic numbers
There was a problem hiding this comment.
why did we change here to magic numbers
it didnt match with steelgain and there was no const for grain. kept it as simple as possible because i wasnt sure which was incorrect, hint or gain
There was a problem hiding this comment.
Toss in a field for it.
Even if the number gets changed later for balance, having a single place to perform that change keeps it simple.
Grain is much more common than any of the refined metals, as long as the trade-in ratio is quite a bit higher than the one for SteelGain, then it will be fine to test with.
fixed duel defeat exit bug fixed teef gain ratio for loot & count stacks bug fix waaagh gain bug respecting sly's comment greenskin realize heroes taste bad
not final (prob)
| _goodAmounts[key] = 0f; | ||
|
|
||
| // including autoresolve battles | ||
| var enemySideEnum = (mapEvent.PlayerSide == BattleSideEnum.Attacker) |
There was a problem hiding this comment.
mapEvent.PlayerSide.GetOppositeSide() is more succinct
| : BattleSideEnum.Attacker; | ||
|
|
||
| var enemySide = mapEvent.GetMapEventSide(enemySideEnum); | ||
| if (enemySide == null) return; |
There was a problem hiding this comment.
Check for a victor. With recent crashes on hideouts, we found that PlayerBattleEndEvent could be dispatched without a victor.
See AddMagicalItemsFromBattle here for the how/why.
| var character = troop.Troop; | ||
| foreach (var ingredientType in ingredientKeys) | ||
| { | ||
| _goodAmounts[ingredientType] += model.GetIngredientDropFactorForCharacter(character, ingredientType, mapEvent); |
There was a problem hiding this comment.
Given the usage, a more apt name would be along the lines of totalIngredientFactors or ingredientFactors as it's not an item count being stored, but a float that will be used later to determine the precise number.
| ); | ||
|
|
||
| foreach (var pair in _goodAmounts) | ||
| var targetRoster = PlayerEncounter.Current?.RosterToReceiveLootItems ?? PartyBase.MainParty.ItemRoster; |
There was a problem hiding this comment.
May be worth logging when items are sent straight to the party's roster, eg. a QuickInformation banner shown on screen when it occurs with a message like 'couldn't find loot roster, depositing to inventory instead' to try and identify the situations where the player is receiving loot, but the game doesn't think they should be.
| var keys = _goodAmounts.Keys.ToList(); | ||
| foreach (var key in keys) _goodAmounts[key] = 0; | ||
| // reset for next battle | ||
| foreach (var key in ingredientKeys) |
There was a problem hiding this comment.
The method also starts off by setting all of the values to 0. Is there a risk that requires it to be set to 0 at both moments?
| { | ||
| private const int ItemExchange = 100; // item of price of X gets X/100 of teef in return | ||
| private const int GoldExchange = 150; | ||
| private const int ItemExchange = 400; // item of price of X gets X/100(now 400) of teef in return. |
There was a problem hiding this comment.
These should be renamed to XEchangeRate since they're used as conversion rates.
Express as "X/400 (previously 100)" if you want the extra info added. The parentheses indicate additional information; if they were to be ignored, the comment should make sense contextually.
| ); | ||
|
|
||
| foreach (var item in Hero.MainHero.PartyBelongedTo.ItemRoster) | ||
| void AfterDonation(ItemRoster beforeSnapshotLocal) |
There was a problem hiding this comment.
Is this handling purely donations, or is the player able to remove previous items that were contributed to the boss' "stash"?
If this is donations only, see
ShrineMenuLogic's menu option
"shrine_menu", "donate"
and it's accompanying OnItemsDiscarded event in ReligionCampaignBehavior to calculate directly from the ItemRoster of donated items and avoid the before/after comparisons.
| if(!mapEvent.IsPlayerMapEvent) return; | ||
|
|
||
| var playerWon = mapEvent.WinningSide == mapEvent.PlayerSide; | ||
| if (mapEvent == null || !mapEvent.IsPlayerMapEvent) return; |
There was a problem hiding this comment.
I lean towards the IsPlayerMapEvent check before the culture check to immediately ignore the majority of map events, but idk if that's actually more performant compared to excluding most playthroughs that don't have a greenskin player.
A reflection moreso than a request for changes.
| Hero.MainHero.AddCustomResource("Waaagh", (int)waaaghGain); | ||
| } | ||
| else | ||
| float ratio = _initialCombatRatio; |
There was a problem hiding this comment.
Is _initialCombatRatio necessary, or using the mapEvent argument with GetStrengthsRelativeToParty(playerSide) allows the ratio to be calculated at this specific moment only?
Does the strength calculation use the parties' initial state before the battle, or the post-battle state?
| if (mapEvent == null || !mapEvent.IsPlayerMapEvent) return; | ||
|
|
||
| if (playerWon) | ||
| if (!IsPlayerLeaderOrInitiator(mapEvent)) |
There was a problem hiding this comment.
What sorts of battles would the player involuntarily participate in where we might want them to not be penalized for a loss that was out of their control (they could have avoided joining if they thought the risk was too high)?
If enlistment is a consideration, that can be explicitly checked for.
Is this to protect against when they have joined an army? If the army loses the battle, 20 Waaagh lost is trivial; the player will more than likely have been taken captive and lost their inventory + party.
Side note, Waaagh should probably be reset to 0 when the player hero is taken prisoner rather than the slow decay that will effectively happen atm.
|
|
||
| bool isGreenskinParty = party.Party?.Culture?.StringId == TORConstants.Cultures.GREENSKIN; | ||
| bool isBanditParty = party.Party?.MobileParty?.IsBandit ?? false; //village defense parties don't have a MobileParty | ||
| bool isBanditParty = party.Party?.MobileParty?.IsBandit ?? false; |
There was a problem hiding this comment.
Hopefully this throws a conflict during merge, but otherwise manually remove it to avoid deleting the comment on that line.
| enemyPartySize += 5; | ||
| enemyPartySize += 4; | ||
| } | ||
| var completeRoster = Settlement.CurrentSettlement.MilitiaPartyComponent.MobileParty.MemberRoster; |
There was a problem hiding this comment.
Not even your change, but why is this line using the militia roster when CanStartBrawl checks the garrison roster?
I'm not sure when militia parties are first created for settlements. I assume it's early enough in campaign initialization to avoid any potential null here.
Militias only contain the basic melee/ranged troops defined in the culture xml; the tier checks performed just below should always have troops below T4 being checked because of the party component used.
| @@ -143,10 +146,29 @@ private void OnMissionEnded(IMission obj) | |||
|
|
|||
| var items = itemIds.Select(id => MBObjectManager.Instance.GetObject<ItemObject>(id)).ToList(); | |||
There was a problem hiding this comment.
The object manager can give a list of objects based on type with GetObjectTypeList.
| && !artifactIds.Contains(i.StringId)) | ||
| .ToList(); | ||
|
|
||
| var cultureItems = MBObjectManager.Instance.GetObjectTypeList<ItemObject>() |
There was a problem hiding this comment.
How does this differ from "items" just above?
I'm curious to see if this will pick up lesser traited loot in the future as a campaign progresses.
| Mission.DoesMissionRequireCivilianEquipment = false; | ||
| _missionAgentSpawnLogic = Mission.GetMissionBehavior<TORMissionAgentHandler>(); | ||
| _missionAgentSpawnLogic.SpawnPlayer(false, true, false, true, false); | ||
| _missionAgentSpawnLogic.SpawnPlayer(true, true, false, true, false); |
There was a problem hiding this comment.
Are the enemies also being spawned with civilian equipment?
Given that all equipment from tor is marked as civilian, does this have a notable effect besides requiring a player to get a 2nd set of gear to equip in their civilian set to effectively have a "battle" set equipped?
A more robust implementation in the future would involve SpawnPlayer (or Spawn(whatever) to submit a different equipment roster to be put on the hero agents).
I've just realized that BrawlMissionController does nothing with playerSideTroops on construction so any companions are effectively "not present". It spawns the player and enemies here, but there's nothing for spawning the player's allies/team, nor a field that stores the player's team's roster.
| if (agent.Team != Mission.PlayerTeam) continue; | ||
| var h = agent.GetHero(); | ||
| var wom = h.GetCustomResourceValue("WindsOfMagic"); | ||
| if (wom != 0) h.AddCustomResource("WindsOfMagic", -(int)wom); |
There was a problem hiding this comment.
Prevent spell use, but allow career abilities?
Or should everything be disabled?
There was a problem hiding this comment.
Does this set someone to 0 WoM when brawling and that will carry over to the campaign map after? Ie. if a future shaman career brawls, will they have to choose between sacrificing all of their WoM and gaining the benefits of the brawl?
| if (party?.LeaderHero != null && party.LeaderHero.GetPerkValue(TORPerks.Faith.Revival)) | ||
| { | ||
| var secondChance = TORPerks.Faith.Revival.PrimaryBonus; | ||
| result = result + (1f - result) * secondChance; // chance to survive if would have died |
There was a problem hiding this comment.
Good use of conditional probabilities.
| super_faction="Kingdom.crooked_eye" | ||
| is_noble="true" | ||
| text="{=str_tor_clan_crooked_eye_clan_1_desc}Facing a fatal defeat against the Dawi-Zharr the Crooked Eye Tribe kicked out their former leader, Ruglud Bonechewer. After a heavy brawl over who should become the next Warboss. A victor appeared; it was Bazglud Daemonstompa. The mad lad had destroyed one of the Daemons sent by the Dawi-Zharr; he had made sure a part of the tribe could escape. Under this new leadership, the tribe restored its numbers and now makes itself ready to make Da Biggest Waaagh! " | ||
| banner_key="11.8.116.1633.1633.764.764.0.1.0.122.20.41.600.600.764.764.0.0.0:kingdom_crookedeye" /> |
There was a problem hiding this comment.
Which one is zed's recent addition and which is the old version?
There was a problem hiding this comment.
This will also need to be verified in the banneroverride xml and the kingdoms xml as all 3 have this overwrite.
SlyDevil
left a comment
SlyDevil
left a comment
There was a problem hiding this comment.
Anything without a comment looks good.
4 participants
a fix to ''mass upgrades in the summarized troop window bypassing restrictions and clicking "done" prevents closing the party menu, but the upgrades have already occurred'', bug. replaced DoneLogic n late checks, they run after PartyScreen queues ops racy, allowed upgrades on ctrl spam and miscounted costs. reserve in AddCommand, affordable applied, clamp TotalNumber, if 0 so it prevents ctrl chains and stale party ui totals.