-
Notifications
You must be signed in to change notification settings - Fork 16
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
Disruptor Upgrade Implement/Model Changes #257
Disruptor Upgrade Implement/Model Changes #257
Conversation
<Tooltip value="RequirementNode/Tooltip/##id##"/> | ||
<OperandArray index="0" value="AP_CountUpgradeAP_DisruptorPerfectedPowerCompleteOnly"/> | ||
</CRequirementNot> | ||
<CRequirementNot id="AP_NotCountUpgradeAP_DisruptorRestrainedDestructionCompleteOnly"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RequirementNodes have messed up ID formatting. Should be AP_NotCountUpgradeDisruptorestrainedDestructionCompleteOnly
. Don't rely on the editor's ID generation, it's bad.
</CRequirementCountUpgrade> | ||
<CRequirementCountUpgrade id="AP_CountUpgradeAP_DisruptorRestructuredThrustersCompleteOnly"> | ||
<Flags index="TechTreeCheat" value="0"/> | ||
<Tooltip value="RequirementNode/Tooltip/##id##"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tooltip values aren't necessary and can be removed; this is already the default. Just make sure the tooltips you add in gamestrings.txt match the updated requirement node IDs.
@@ -8897,6 +8897,11 @@ | |||
<TechPurchaseCamera value="Star2CameraLow15"/> | |||
<UnitGlossaryCamera value="Star2CameraLow15"/> | |||
</CModel> | |||
<CModel id="ImmortalAttackPurifierImpact" parent="ImpactFX"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This model should match our ID naming scheme with an AP_
prefix.
<EffectArray Reference="Effect,AP_PurificationNovaEnemyDamage,Amount" Value="55"/> | ||
<EffectArray Reference="Effect,AP_PurificationNovaFriendlyDamage,Amount" Value="27.5"/> | ||
<EffectArray Operation="Subtract" Reference="Effect,AP_PurificationNovaDamage,ShieldBonus" Value="55"/> | ||
<EffectArray Operation="Subtract" Reference="Effect,AP_PurificationNovaFriendlyDamage,ShieldBonus" Value="27.5"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The increase in damage to friendlies when you have restrained distruction can be construed as a nerf. I think this upgrade should just not touch the friendly-fire damage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally I just wanted to keep consistency in damage, at first I didn't think this would be much of a nerf if any but after thinking about it more I can see how this would be a problem in certain scenarios. Increased friendly-fire damage removed.
Made fixes as per Phaneros' suggestions, and added extra animation to disruptor based on if perfected power upgrades is unlocked (to show explosion hitting air units). Comparison with colossus size as reference: Untitled.video.-.Made.with.Clipchamp.22.mp4 |
Noting this is progress on #152 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good; note I haven't tested this, just reviewing code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once sec, didn't notice this file being added. Is this file not available through liberty.sc2mod, and hence doesn't need to be added here? Looking through the game files really quite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's in liberty.sc2mod at location base.sc2assets/ assets/effects/protoss/adept_collection_purifier_ricochetglaive_missile_impact/adept_collection_purifier_ricochetglaive_missile_impact.m3
. Should be removed from the PR and references should use the version in the game files.
<EditorCategories value="Race:Protoss"/> | ||
</CModel> | ||
<CModel id="AP_PerfectedPowerNova" parent="OneShotSpellFX"> | ||
<Model value="Assets\Effects\Protoss\Adept_Collection_Purifier_RicochetGlaive_Missile_Impact\Adept_Collection_Purifier_RicochetGlaive_Missile_Impact.m3"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This references the value in the gamefiles (no AP/
prefix) and thus doesn't have to be updated. The file should not be added, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for catching that, I thought I removed that file in my recent pull requests but appears I forgot to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the .m3 file from the PR; it is not necessary and bloats the download.
That string implies you should add the |
### Changes Made:
Base Unit
For base unit, model was changed to a purifier version of the model. It was changed in order to more represent the fact that it is part of the purifier faction:
Untitled.video.-.Made.with.Clipchamp.10.mp4
Warping in from a Robotics Facility (Tooltip stayed the same, just image and model placement difference)
Untitled.video.-.Made.with.Clipchamp.12.mp4
Upgrades
Cloaking Module : Disruptor is permanently cloaked. Basically works the same as other permanent-cloak upgrades.
Untitled.video.-.Made.with.Clipchamp.13.mp4
Restructured Thrusters : Allows the Disruptor to move while casting Purification Nova. Also allows the Disruptor to Blink. This ability does NOT Blink alongside similar casts, such as Stalker or Cloaked Warrior Blinks.
Main reasoning behind this is to avoid a situation where you blink units such as slayers/stalkers/cloaked warriors closer to the enemy but your disruptors get in grave danger if they blinked alongside. Also want to preserve the idea Disruptors are micro-heavy units.
Untitled.video.-.Made.with.Clipchamp.14.mp4
Untitled.video.-.Made.with.Clipchamp.15.mp4
Tooltips (Purification Nova has immobile part of tooltip removed):
Perfected Power : Allows Purification Nova to hit air units. Purification Nova now always deals 200 damage, regardless of the target has shields or not. Note that this includes ally-air units.
Untitled.video.-.Made.with.Clipchamp.16.mp4
Tooltips (Orange Purification Nova in image is when both Restructured Thrusters and Perfected Power are active):
Restrained Destruction : Purification Nova does 50% reduced damage to friendly units and structures. Takes into account if Perfected Power is active or not.
Untitled.video.-.Made.with.Clipchamp.17.mp4
Tooltip:
Gameplay Example Clips (yes I'm bad at using these units):
Untitled.video.-.Made.with.Clipchamp.19.mp4
Untitled.video.-.Made.with.Clipchamp.21.mp4
War Council
If anything can be done to make these upgrades more applicable for the War Council, please let me know. In my opinion, either Restructured Thrusters or Perfected Power would work best as the War Council upgrades; I don't think the base unit is good enough to have it be nerfed by a significant amount. If it would be nerfed from it's baseline variation, I would honestly just argue for it to cost more minerals/gas/supply.
EDIT: Changed Disruptor Upgrade "Restructured Thrusters" to be it's War Council upgrade.
Archipelago Item Pull Request: Ziktofel/Archipelago#283