Skip to content

PeriodicWeapon#2216

Open
AGAGUGUG wants to merge 9 commits into
Phobos-developers:developfrom
AGAGUGUG:develop
Open

PeriodicWeapon#2216
AGAGUGUG wants to merge 9 commits into
Phobos-developers:developfrom
AGAGUGUG:develop

Conversation

@AGAGUGUG
Copy link
Copy Markdown

The function of this property is to periodically fire weapons at all targets within its range. Currently, it determines which targets are locked by polling the units within its range. Currently, there are the following key-value pairs PeriodicWeapon= PeriodicWeapon.FiringDelay=
PeriodicWeapon.Range=
PeriodicWeapon.AffectsHouses=
PeriodicWeapon.UseInvokerAsOwner=
PeriodicWeapon.FireAll=
PeriodicWeapon.AffectTypes=
PeriodicWeapon.IgnoreTypes=
When the value is 'no', it will only fire at the nearest target within range that can be filtered; when this value is 'yes', it will fire at all targets within range.

@TaranDahl
Copy link
Copy Markdown
Contributor

Docs needed

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 17, 2026

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.

The function of this property is to periodically fire weapons at all targets within its range. Currently, it determines which targets are locked by polling the units within its range. Currently, there are the following key-value pairs PeriodicWeapon=
PeriodicWeapon.FiringDelay=
PeriodicWeapon.Range=
PeriodicWeapon.AffectsHouses=
PeriodicWeapon.UseInvokerAsOwner=
PeriodicWeapon.FireAll=
PeriodicWeapon.AffectTypes=
PeriodicWeapon.IgnoreTypes=
When the value is 'no', it will only fire at the nearest target within range that can be filtered; when this value is 'yes', it will fire at all targets within range.
@phoboscn-bot
Copy link
Copy Markdown

To Chinese users:
This pull request has been mentioned on Phobos CN. There might be relevant details there:

致中文用户:
此拉取请求已在 Phobos CN 上被提及。那里可能有相关详细信息:

https://www.phoboscn.top/t/topic/467/4

Copy link
Copy Markdown
Contributor

@TaranDahl TaranDahl left a comment

Choose a reason for hiding this comment

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

Comment thread docs/New-or-Enhanced-Logics.md Outdated
Comment thread docs/New-or-Enhanced-Logics.md Outdated
Comment thread docs/New-or-Enhanced-Logics.md Outdated
Comment thread docs/New-or-Enhanced-Logics.md Outdated
Comment thread docs/New-or-Enhanced-Logics.md Outdated
Comment thread src/Ext/Techno/WeaponHelpers.cpp Outdated
Comment thread src/New/Entity/AttachEffectClass.cpp Outdated
Comment thread src/New/Entity/AttachEffectClass.cpp Outdated
Comment thread src/New/Entity/AttachEffectClass.cpp Outdated
@TaranDahl TaranDahl added ❓New feature Needs improvement ⚙️T2 T2 maintainer review is sufficient labels May 18, 2026
- Renamed: `FiringDelay` → `Delay`, added `InitialDelay` for first-shot delay
- Removed obsolete fields: `AffectsHouse`, `AffectTypes`, `IgnoreTypes`, `FireAll`
- Added target selection strategy: `Target = nearest / all / random`
- Legacy keys `FiringDelay` and `FireAll` are still readable but will trigger deprecation warnings (same as `RevengeWeapon`)

- Targeting logic moved to `WeaponHelpers.cpp`, shared between `Splits` and `PeriodicWeapon`
- Removed duplicate `IsProjectileEligibleTarget`
- Target filtering now uses weapon-side rules: `CanTargetHouses`, `CanTarget`, health/veterancy, AE requirements, etc.
- Self-targeting is now controlled by projectile flag `RetargetSelf` (no longer hard-excludes the attach owner)

- `FirePeriodicWeapon` now respects firepower multiplier, sorts targets by distance
- Updated documentation: `docs/New-or-Enhanced-Logics.md` and Chinese po file
@AGAGUGUG
Copy link
Copy Markdown
Author

Preliminary modifications have been made

1 similar comment
@AGAGUGUG

This comment was marked as duplicate.

AGAGUGUG added 2 commits May 19, 2026 16:01
- Renamed: `FiringDelay` → `Delay`, added `InitialDelay` for first-shot delay
- Removed obsolete fields: `AffectsHouse`, `AffectTypes`, `IgnoreTypes`, `FireAll`
- Added target selection strategy: `Target = nearest / all / random`
- Legacy keys `FiringDelay` and `FireAll` are still readable but will trigger deprecation warnings (same as `RevengeWeapon`)

- Targeting logic moved to `WeaponHelpers.cpp`, shared between `Splits` and `PeriodicWeapon`
- Removed duplicate `IsProjectileEligibleTarget`
- Target filtering now uses weapon-side rules: `CanTargetHouses`, `CanTarget`, health/veterancy, AE requirements, etc.
- Self-targeting is now controlled by projectile flag `RetargetSelf` (no longer hard-excludes the attach owner)

- `FirePeriodicWeapon` now respects firepower multiplier, sorts targets by distance
- Updated documentation: `docs/New-or-Enhanced-Logics.md` and Chinese po file
PeriodicWeapon.FiringDelay
PeriodicWeapon.Target
PeriodicWeapon.FireAll
has been delete

Currently, only the following key values are available

PeriodicWeapon=YourWeaponName              ; WeaponType
PeriodicWeapon.Delay=30                    ; integer - game frames
PeriodicWeapon.InitialDelay=15             ; integer - game frames
PeriodicWeapon.Range=5                     ; floating point, cells
PeriodicWeapon.TargetingMode=closest       ; closest | all | <callback name>
PeriodicWeapon.TargetSelf=false            ; boolean
PeriodicWeapon.UseInvokerAsOwner=false     ; boolean
Comment thread Phobos.log Outdated
Comment thread src/Ext/Techno/Body.h Outdated
Comment thread src/Ext/Techno/Body.h Outdated
AGAGUGUG added 2 commits May 19, 2026 22:43
Refactored split/periodic target validation into WeaponTypeExt::IsAllowedTarget and BulletTypeExt::IsAllowedTarget with no intended behavioral change; call sites updated for splits and periodic weapon.
@AGAGUGUG
Copy link
Copy Markdown
Author

Done

Comment thread src/RCa55180 Outdated
@AGAGUGUG
Copy link
Copy Markdown
Author

So is there anything else that needs to be changed?

@TaranDahl
Copy link
Copy Markdown
Contributor

So is there anything else that needs to be changed?

Will review this on Friday. I am not available now.

@AGAGUGUG
Copy link
Copy Markdown
Author

OK I got it

@TaranDahl
Copy link
Copy Markdown
Contributor

Besides, since this is your first contribution, it needs to be reviewed by senior maintainers in accordance with the process.
@ZivDero maybe you can take a look?

Comment thread docs/New-or-Enhanced-Logics.md Outdated
PeriodicWeapon= ; WeaponType
PeriodicWeapon.Delay=0 ; integer - game frames
PeriodicWeapon.InitialDelay=0 ; integer - game frames
PeriodicWeapon.Range=0 ; floating point value, distance in cells
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the need for a separate range? Why should the weapon's range not be used?

Comment thread docs/New-or-Enhanced-Logics.md Outdated
Comment on lines +209 to +230
#### Periodic weapon custom targeting

Modders can register custom target selection logic in code for use with `PeriodicWeapon.TargetingMode=<name>`:

```cpp
#include <New/PeriodicWeaponTargeting.h>

static std::vector<TechnoClass*> MyTargeting(
AttachEffectClass* pAttachEffect, TechnoClass* pAttached,
WeaponTypeClass* pWeapon, TechnoClass* pFirer, HouseClass* pFirerHouse,
const PeriodicWeaponValidTargetList& validTargets)
{
// validTargets is sorted by distance (nearest first)
// return technos to fire at this interval; empty vector skips firing
}

// Register during init, e.g. from a DEFINE_HOOK
PeriodicWeaponTargeting::Register("MyMode", MyTargeting);
```

Target search uses `BulletTypeExt::IsAllowedTarget` and `WeaponTypeExt::IsAllowedTarget` with full weapon targeting enabled.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

C++ code in docs? WUT?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No bother. I will move the callback things to the API doc when the API PR is finished.

Comment thread src/New/Entity/AttachEffectClass.cpp Outdated
Comment on lines +632 to +641
TechnoClass* pFirer = pType->PeriodicWeapon_UseInvokerAsOwner ? this->Invoker : pTechno;
HouseClass* pFirerHouse = pType->PeriodicWeapon_UseInvokerAsOwner
? (this->InvokerHouse ? this->InvokerHouse : pTechno->Owner)
: pTechno->Owner;

if (!pFirer)
pFirer = pTechno;

if (!pFirerHouse)
pFirerHouse = pTechno->Owner;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
TechnoClass* pFirer = pType->PeriodicWeapon_UseInvokerAsOwner ? this->Invoker : pTechno;
HouseClass* pFirerHouse = pType->PeriodicWeapon_UseInvokerAsOwner
? (this->InvokerHouse ? this->InvokerHouse : pTechno->Owner)
: pTechno->Owner;
if (!pFirer)
pFirer = pTechno;
if (!pFirerHouse)
pFirerHouse = pTechno->Owner;
const bool useInvoker = pType->PeriodicWeapon_UseInvokerAsOwner;
TechnoClass* pFirer = useInvoker && this->Invoker
? this->Invoker
: pTechno;
HouseClass* pFirerHouse = useInvoker && this->InvokerHouse
? this->InvokerHouse
: pTechno->Owner;

Comment thread src/New/Entity/AttachEffectClass.cpp Outdated
Comment on lines +650 to +673
for (auto const pTarget : TechnoClass::Array)
{
if (!pTarget || pTarget->InLimbo)
continue;

if (!targetSelf && pTarget == pTechno)
continue;

if (!pTarget->IsInPlayfield || !pTarget->IsOnMap || !pTarget->IsAlive || pTarget->Health <= 0)
continue;

const int dist = pTarget->DistanceFrom(pTechno);

if (dist > searchRange)
continue;

if (!BulletTypeExt::IsAllowedTarget(pBulletType, pTarget, useWeaponTargeting, pFirer))
continue;

if (!WeaponTypeExt::IsAllowedTarget(pWeapon, pTarget, useWeaponTargeting, pFirer, pFirerHouse))
continue;

validTargets.emplace_back(pTarget, dist);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not the end of the world, to iterate all technos, but you could consider iterating cells' occipiers + using AircraftTrackerClass

Comment thread src/New/Entity/AttachEffectClass.cpp Outdated
Comment on lines +701 to +709
if (!_strcmpi(mode, PeriodicWeaponTargeting::ModeAll))
{
for (auto const& [pTarget, _] : validTargets)
fireAt(pTarget);
}
else if (!_strcmpi(mode, PeriodicWeaponTargeting::ModeClosest))
{
fireAt(validTargets.front().first);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Uh.... No. Parse the mode while reading the INI and save it as an enum, don't do string comparisons when firing.

@ZivDero
Copy link
Copy Markdown
Contributor

ZivDero commented May 20, 2026

The whole "modders can make new modes" needs to be thoroghly elaborated on, and considered after that.

@TaranDahl
Copy link
Copy Markdown
Contributor

The whole "modders can make new modes" needs to be thoroghly elaborated on, and considered after that.

I thought the callback thing was already discussed in 2131?

1.Now PeriodicWeapon.Range will no longer be used, all range-related code will read the properties of the weapon itself
2.Modify the enemy-locking related code, now it will no longer scan all units on the map, only units within the attack range will be scanned
3.Since the current method of mounting weapons uses InRange logic, some native weapon-related key values may not be effective, for example, using a cylindrical range to detect enemies.
@ZivDero
Copy link
Copy Markdown
Contributor

ZivDero commented May 20, 2026

The whole "modders can make new modes" needs to be thoroghly elaborated on, and considered after that.

I thought the callback thing was already discussed in 2131?

Ah.... If that's in relation to that I have no objections. Though it's not exactly a modder-facing feature, but that's a docs nitpick.

Then storing the type as a string sorta makes sense, too, if we haven't got a closed list of options.

@TaranDahl
Copy link
Copy Markdown
Contributor

Then storing the type as a string sorta makes sense, too, if we haven't got a closed list of options.

I think we can try parsing it as both a callback name and an enum at the same time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs improvement ❓New feature ⚙️T2 T2 maintainer review is sufficient

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants