-
-
Notifications
You must be signed in to change notification settings - Fork 103
[MultiWeapon] Use WeaponX #1630
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
base: develop
Are you sure you want to change the base?
Conversation
Nightly build for this pull request:
This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build. |
|
||
const auto pTypeExt = TechnoTypeExt::ExtMap.Find(pThis->GetTechnoType()); | ||
|
||
if (pTypeExt && pTypeExt->MultiWeapon.Get() && |
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.
no need to do sanity check for pTypeExt
bool TechnoExt::CheckMultiWeapon(TechnoClass* const pThis, AbstractClass* const pTarget, WeaponTypeClass* const pWeaponType) | ||
{ | ||
if (!pThis || !pWeaponType || pWeaponType->NeverUse || | ||
!pWeaponType->Projectile || !pWeaponType->Warhead || |
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.
no need to check if Projectile or Warhead exist since they'll still crash elsewhere
if (pTargetTechno->Health <= 0 || !pTargetTechno->IsAlive) | ||
return false; | ||
|
||
if (const auto pTargetExt = TechnoExt::ExtMap.Find(pTargetTechno)) |
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.
same, no need to do sanity check for pTargetExt
} | ||
|
||
if (GeneralUtils::GetWarheadVersusArmor(pWH, pTargetTechno->GetTechnoType()->Armor) == 0.0) | ||
return false; |
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 is wrong when having a shield with a different armor that could be targeted. Should check it separately
if (pShield || pShield->IsActive())
{ // shield checking }
else if (GeneralUtils::GetWarheadVersusArmor(pWH, pTargetTechno->GetTechnoType()->Armor) == 0.0)
{ return false; }
|
||
const auto pBld = abstract_cast<BuildingClass*>(pTargetTechno); | ||
|
||
if (!pBld || !pBld->Type || !pBld->Type->Overpowerable) |
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.
no need to check if pBld->Type exists
@@ -996,6 +996,11 @@ void TechnoTypeExt::ExtData::Serialize(T& Stm) | |||
.Process(this->Overload_DeathSound) | |||
.Process(this->Overload_ParticleSys) | |||
.Process(this->Overload_ParticleSysCount) | |||
|
|||
.Process(this->MultiWeapon) |
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.
wrong spacing
INI_EX exINI(pINI); | ||
const char* pSection = pThis->ID; | ||
|
||
if (const auto pTypeExt = TechnoTypeExt::ExtMap.Find(pThis)) |
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.
since there's pThis->ID above, I'm assuming pThis will always be not null. In this case this sanity check is not needed
GET(TechnoTypeClass*, pThis, EBP); | ||
enum { ReadWeaponX = 0x715B1F, Continue = 0x715B17 }; | ||
|
||
if (const auto pTypeExt = TechnoTypeExt::ExtMap.Find(pThis)) |
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.
check if pThis here is always not null too
} | ||
|
||
// maybe it could be used as a new SelectWeapon, but not for now. | ||
bool TechnoExt::CheckMultiWeapon(TechnoClass* const pThis, AbstractClass* const pTarget, WeaponTypeClass* const pWeaponType) |
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.
It seems that this function is not being used yet?
if (pWH->Parasite && (pTargetTechno->WhatAmI() == AbstractType::Building || | ||
abstract_cast<FootClass*>(pTargetTechno)->ParasiteEatingMe)) | ||
return false; | ||
|
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.
actually, maybe armor versus check can be put to here to save as many as calculations
Technology types are free to enable WeaponX and use them as usable weapons.
WeaponX
as their weapon whenMultiWeapon=yes
, be careful not to forgetWeaponCount
.MultiWeapon.IsSecondary
can only be used for infantry and is responsible for determining which weapons should useSecondaryFire
in theSequence
.In
rulesmd.ini
:科技类型可以自由启用WeaponX并将它们作为可用武器使用。
MultiWeapon=yes
时科技类型会读取WeaponX
作为它的武器,同时注意不要忘记写WeaponCount
。MultiWeapon.IsSecondary
只对步兵有效,它会负责判断哪些武器开火时播放副武器动作帧。