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 Tyrannozor Merge and Upgrades for Tyrannozor/Ultralisk from Dehaka Co-op #217

Merged
merged 42 commits into from
Jul 29, 2024

Conversation

itsjustbones
Copy link

##What is this fixing or adding?

Adds Tyrannozor Merge to Ultralisk and Morphling
Barrage of Spikes Upgrade for Tyrannozor - Tyrannozors can unleash a Barrage of Spikes, dealing 100 damage to nearby enemy ground and air units.
Tyrant's Protection Upgrade for Tyrannozor - Tyrannozors grants nearby friendly units 2 armor.
Imapling Strike Upgrade for Ultralisk/Tyrannozor - Ultralisk and Tyrannozor melee attacks have a 20% chance to stun for 2 seconds.
Healing Adaption Upgrade for Ultarlisk/Tyrannozor - Ultralisks and Tyrannozors regenerate life quickly when out of combat.

Also, due to command card space, this moves 3 Passive upgrades from Ultralisk command card to behaivor icons, Anabolic Synthesis, Chitinous Plating, Monarch Blades

Tied to Ziktofel/Archipelago#251

SC2_x64_ANCu2cevFd
SC2_x64_azEaonTfQx
SC2_x64_eOQvYkvuLe
SC2_x64_7sIwzgmzlw
SC2_x64_GVaXfMIPGx
SC2_x64_RWvWdXr1Ja
SC2_x64_YNSOzPuTdy
SC2_x64_PZJ7EeK9mJ
SC2_x64_Ian3e81TIP

Please note that this is my first PR for both SC2 AP and in general, so if I've messed anything up or you have recommendations, please don't hesitate to let me know.

@@ -7,7 +7,6 @@
<Actor id="AP_SOAGravitonBombTargetSound"/>
<Actor id="AP_SOAOrbitalStrikeTargetModel"/>
<Actor id="AP_SOATargetingModeConfirmationSound"/>
<Actor id="AP_Stargate"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if the editor removed this or if you did.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember seeing this when resolving merge conflicts, I must have done something wrong when fixing conflicts, but I can't remember what. It did stick out to me as odd though.

@@ -4794,6 +4794,30 @@ void libABFE498B_gf_AP_Triggers_Zerg_unlockSporeCrawler (int lp_player) {
TechTreeUnitAllow(lp_player, "AP_SporeCrawler", true);
}

void libABFE498B_gf_AP_Triggers_Zerg_TyrannozorHealingAdapation (int lp_player) {
Copy link
Contributor

@MatthewMarinets MatthewMarinets Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo -- "Adapation" -> "Adaptation"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

<CValidatorPlayerRequirement id="AP_HaveTyrannozorMerge">
<Find value="1"/>
<Value value="AP_HaveTyrannozorMerge"/>
</CValidatorPlayerRequirement>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validator seems unused?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, no idea why I even wrote this, probably some throw it at the wall idea for fixing merges that I forgot to cleanup

<CValidatorPlayerRequirement id="AP_HaveTyrannozorBarrageOfSpikes">
<Find value="1"/>
<Value value="AP_HaveTyrannozorBarrageOfSpikes"/>
</CValidatorPlayerRequirement>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validator seems unused.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, stems from my misunderstanding of validators and shouldn't be necessary

<CValidatorPlayerRequirement id="AP_HaveHealingAdaption">
<Find value="1"/>
<Value value="AP_HaveHealingAdaption"/>
</CValidatorPlayerRequirement>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validator seems unused -- only the requirement is used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, stems from my misunderstanding of validators and shouldn't be necessary

<CValidatorPlayerRequirement id="AP_HaveMonarchBlades">
<Find value="1"/>
<Value value="AP_HaveUltraliskOrganicCarapace"/>
</CValidatorPlayerRequirement>
Copy link
Contributor

@MatthewMarinets MatthewMarinets Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validator is unused, and doesn't seem to be checking what the name implies

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, bad copying and forgetting to remove or edit.

@itsjustbones
Copy link
Author

I've retested to make sure nothing broke as well.

Comment on lines +11913 to +11914
<EffectArray Operation="Subtract" Reference="Unit,AP_Tyrannozor,CostResource[Minerals]" Value="200"/>
<EffectArray Reference="Unit,AP_Tyrannozor,Food" Value="3"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to include the burrowed Tyrannozor in this too, otherwise it'll cost food to burrow

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks for catching

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snímek obrazovky pořízený 2024-07-26 19-55-23
Minerals too

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@Ziktofel
Copy link
Owner

Ziktofel commented Jul 26, 2024

Bug: Air attack sound doesn't play, no unit response sounds

(you might be linking a sound file from a dependency that isn't a dependency of ArchipelagoPlayer)

@@ -396,6 +397,8 @@ Button/Hotkey/AP_TimeWarp=C
Button/Hotkey/AP_TorrasqueChrysalis=Q
Button/Hotkey/AP_TrainHERC=H
Button/Hotkey/AP_TransportMode=T
Button/Hotkey/AP_TyrannozorBarrageOfSpikes=Q
Button/Hotkey/AP_TyrannozorMerge=G
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd pick T as the hotkey (T for Tyrannozor)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason I thought T was a hotkey for a default action, like patrol, and just didn't check. I've updated it.

@Ziktofel
Copy link
Owner

Fix the following warnings:

[7/26/2024 6:46:55 PM] Warning: XML: StarCraft II\Mods\ArchipelagoPlayer.SC2Mod\Base.SC2Data\GameData\SoundData.xml(11637,5) : Unable to find parent HoS_PrimalAck  Source: Mods/ArchipelagoPlayer.SC2Mod  Scope: CSound
[7/26/2024 6:46:55 PM] Warning: XML: StarCraft II\Mods\ArchipelagoPlayer.SC2Mod\Base.SC2Data\GameData\SoundData.xml(11709,5) : Unable to find parent HoS_PrimalAck  Source: Mods/ArchipelagoPlayer.SC2Mod  Scope: CSound
[7/26/2024 6:46:55 PM] Warning: XML: StarCraft II\Mods\ArchipelagoPlayer.SC2Mod\Base.SC2Data\GameData\SoundData.xml(11743,5) : Unable to find parent HoS_PrimalAck  Source: Mods/ArchipelagoPlayer.SC2Mod  Scope: CSound
[7/26/2024 6:46:55 PM] Warning: XML: StarCraft II\Mods\ArchipelagoPlayer.SC2Mod\Base.SC2Data\GameData\SoundData.xml(11766,5) : Unable to find parent HoS_PrimalAck  Source: Mods/ArchipelagoPlayer.SC2Mod  Scope: CSound

@itsjustbones
Copy link
Author

Bug: Air attack sound doesn't play, no unit response sounds

(you might be linking a sound file from a dependency that isn't a dependency of ArchipelagoPlayer)

I did my sound checks on The reckoning, which is a hots map, so I'm guessing that map has the swarm.sc2campaign dependency, but since the mod doesn't, it breaks on other non hots maps. I'll have to upload it separately then.

Comment on lines 12844 to 12853
<EffectArray Reference="Unit,AP_Ultralisk,LifeRegenRate" Value="10.000000"/>
<EffectArray Reference="Unit,AP_UltraliskBurrowed,LifeRegenRate" Value="10.000000"/>
<EffectArray Reference="Unit,AP_UltraliskResourceEfficiency,LifeRegenRate" Value="10.000000"/>
<EffectArray Reference="Unit,AP_Ultralisk,LifeRegenDelay" Value="10.000000"/>
<EffectArray Reference="Unit,AP_UltraliskBurrowed,LifeRegenDelay" Value="10.000000"/>
<EffectArray Reference="Unit,AP_UltraliskResourceEfficiency,LifeRegenDelay" Value="10.000000"/>
<EffectArray Reference="Unit,AP_Tyrannozor,LifeRegenRate" Value="10.000000"/>
<EffectArray Reference="Unit,AP_TyrannozorBurrowed,LifeRegenRate" Value="10.000000"/>
<EffectArray Reference="Unit,AP_Tyrannozor,LifeRegenDelay" Value="10.000000"/>
<EffectArray Reference="Unit,AP_TyrannozorBurrowed,LifeRegenDelay" Value="10.000000"/>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Stops regular life regen (that works even in combat). Look into Regen Biosteel code for Infested Tanks/Banshees that allows basic regen (and increased regen while out of combat)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Though I'll note that I just copied vanilla on this originally, which is apparently kinda dumb.

Comment on lines 24215 to 24218
<LayoutButtons Face="AP_TyrannozorBarrageOfSpikes" Type="AbilCmd" AbilCmd="AP_TyrannozorBarrageOfSpikes,Execute" Row="2" Column="0"/>
<LayoutButtons Face="AP_HealingAdaptation" Type="Passive" Requirements="AP_HaveHealingAdaptation" Row="2" Column="1"/>
<LayoutButtons Face="AP_TyrannozorTyrantsProtection" Type="Passive" Requirements="AP_HaveTyrannozorTyrantsProtection" Row="2" Column="2"/>
<LayoutButtons Face="AP_ImpalingStrike" Type="Passive" Requirements="AP_HaveImpalingStrike" Row="2" Column="3"/>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally we put passives on the second row

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@itsjustbones
Copy link
Author

Sounds should be fixed now

Comment on lines +11913 to +11914
<EffectArray Operation="Subtract" Reference="Unit,AP_Tyrannozor,CostResource[Minerals]" Value="200"/>
<EffectArray Reference="Unit,AP_Tyrannozor,Food" Value="3"/>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snímek obrazovky pořízený 2024-07-26 19-55-23
Minerals too

@Ziktofel
Copy link
Owner

Add

<CActorSplat id="AP_TyrannozorBurrowedSplat" parent="BurrowedSplat" unitName="AP_Tyrannozor"/>

to actors for correct burrow display

@itsjustbones itsjustbones requested a review from Ziktofel July 27, 2024 22:09
Comment on lines 5655 to 5660
<CValidatorPlayerCompareFoodAvailable id="AP_CheckTyrannozorMorphlingMergeSupplyResourceEfficiency">
<ResultFailed value="NotEnoughFood"/>
<WhichPlayer Value="Caster"/>
<Compare value="LT"/>
<Value value="3"/>
</CValidatorPlayerCompareFoodAvailable>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 2 supply (Tyrannozor is 6 supply if upgraded, morphlings are 2 supply each)
Snímek obrazovky pořízený 2024-07-28 18-51-29
I have enough supply available but still can't merge

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment on lines 24221 to 24223
<LayoutButtons Face="AP_HealingAdaptation" Type="Passive" Requirements="AP_HaveHealingAdaptation" Row="1" Column="1"/>
<LayoutButtons Face="AP_TyrannozorTyrantsProtection" Type="Passive" Requirements="AP_HaveTyrannozorTyrantsProtection" Row="1" Column="2"/>
<LayoutButtons Face="AP_ImpalingStrike" Type="Passive" Requirements="AP_HaveImpalingStrike" Row="1" Column="3"/>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move the upgrade icons in order to start from the first column (index 0)? Frenzied should stay in place

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, wasn't sure about this, I figured it just looked better to have them all next to each other, but didn't want to move frenzied

@itsjustbones itsjustbones requested a review from Ziktofel July 28, 2024 20:07
@Ziktofel Ziktofel merged commit 561ae04 into Ziktofel:sc2-next Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants