Pseudo Illusions System

[Wrda]

Initialization is a critical part of a map. I fear your initialization on Lua root will cause desyncs. Either use Total Initialization from Bribe (OnInit.map function) or hook into something such as InitBlizzard. Example:

local oldInit = InitBlizzard
function InitBlizzard()
    oldInit()
    --initialize
end

Unit Index Manager script:
I don't understand the use of OOP here, you're not creating multiple instances, and the getInstance method has no variance.
redundant checks: UnitIndexManager:OnUnitEnter already checks if unit is nil, no need to check it again on UnitIndexManager:AssignId.
How can a triggered event be fired if the "unit" variable, with the event response, is nil? Besides that, I've never tested if creating a corpse triggers this event.

local id = GetUnitUserData(unit) this will never be nil, unless someone goes out of their way to replace the function.

Unit Indexer is going to fail if units are revived. It should be done when unit is about to be removed from the game, which can be detected by the undefend order bug.
Generally modifying GetUnitUserData isn't allowed unless this resource was a Unit Indexer system. The reason is there's already existing Unit Indexers and they most likely provide a better implementation than this one and the user is using one of them. Even so, in Lua it isn't really needed, since you can make a table, storing the unit itself as a key, with whatever value.

I took a glance at Direct Dynamic Damage Detection and I don't see its relevance for the system the resource is about. Please move it to its own folder so it doesn't get confused with the scripts that the system requires, and the system itself.
[In Addition, why does damage detection system has a GetUnitZ function?]

-- Create a default instance of the system
local illusionsInstance = PseudoIllusionsSystem:GetInstance(
    FourCC('API0'),  -- Ability ID to mark illusions
    FourCC('API1'),  -- Spell ID that creates illusions
    "metamorphosis", -- Buff order string
    FourCC('API2'),  -- Buff ID to disable
    60.0,            -- Illusion duration (seconds)
    200              -- Spawn offset distance
)

This would seem more appropriate as an example, and not inherent to the system.

--[[
    Handles item pickup attempts by illusions (prevents them from taking items)
    @param illusionData: The data table for the illusion attempting pickup
--]]
function PseudoIllusionsSystem:OnUnitPickupItem(illusionData)
    if not illusionData or not illusionData.triggeringUnit then return end

    local illusionUnit = illusionData.triggeringUnit
    local item = GetManipulatedItem()
    if not item then return end

    -- Drop the item back where it was picked up
    SetItemPosition(item, GetUnitX(illusionUnit), GetUnitY(illusionUnit))
end

The "illusion" already can't pickup items since it has the ability which locks the inventory, this function is redundant.
You have similar redundant checks:
if not GetEventDamageSource() or not BlzGetEventDamageTarget() then return end
It's not possible for a non-existing unit to be damaged, so the 2nd operand isn't needed.

GetUnitX(target) + self.illusionOffset * Cos(GetRandomReal(0, 6.283)),
    GetUnitY(target) + self.illusionOffset * Sin(GetRandomReal(0, 6.283)),

That number is bj_PI*2 (TAU), you should use it.

It doesn't look like the system accounts for units with Reincarnation ability.
I think you may also have to copy the damage "number of dice" and "sides per die" onto the illusion.
Item position becomes always slot-ordered if the hero happens to have some empty slots while having an item at any slot from 2-5. It's a minor issue. If you're trying to mimic the standard illusions, then it's missing the change of unit vertex colour for allies, besides the owner of illusion. Even with experience disabled I received less exp than I should have. I've got 85 exp without illusions, 43 exp with 1 illusion. Furthermore, the illusions are currently giving exp points to enemies, and bounty if they happen to be killed, and if bounty is enabled.

I think you have a nice idea and the code itself isn't bad, but it needs some reworks in several aspects, such as:
More configurables, such as the special effect paths for the creation and the destruction of the illusions.
A clearer API: PseudoIllusionsSystem.Create(sourceUnit, illusionId, spawnOffset, damagedFactor, attackDamageFactor) -> unit
PseudoIllusionsSystem.IsIllusion(unit) -> boolean
Doesn't need to have these exact names of course.
Is it easier for the user to input the illusion Id version themselves or the system can do it itself? I don't know. However, I wouldn't risk the FourCC and reverse functions you made, specially since blizzard's function may fail in some edge case (Luashine discovered this).

And this was mostly about the Lua version. Next is the vJASS (you have JASS tag instead of vJASS )

 

[Glucose]

Initialization is a critical part of a map. I fear your initialization on Lua root will cause desyncs. Either use Total Initialization from Bribe (OnInit.map function) or hook into something such as InitBlizzard. Example:

local oldInit = InitBlizzard
function InitBlizzard()
    oldInit()
    --initialize
end

Unit Index Manager script:
I don't understand the use of OOP here, you're not creating multiple instances, and the getInstance method has no variance.
redundant checks: UnitIndexManager:OnUnitEnter already checks if unit is nil, no need to check it again on UnitIndexManager:AssignId.
How can a triggered event be fired if the "unit" variable, with the event response, is nil? Besides that, I've never tested if creating a corpse triggers this event.

local id = GetUnitUserData(unit) this will never be nil, unless someone goes out of their way to replace the function.

Unit Indexer is going to fail if units are revived. It should be done when unit is about to be removed from the game, which can be detected by the undefend order bug.
Generally modifying GetUnitUserData isn't allowed unless this resource was a Unit Indexer system. The reason is there's already existing Unit Indexers and they most likely provide a better implementation than this one and the user is using one of them. Even so, in Lua it isn't really needed, since you can make a table, storing the unit itself as a key, with whatever value.

I took a glance at Direct Dynamic Damage Detection and I don't see its relevance for the system the resource is about. Please move it to its own folder so it doesn't get confused with the scripts that the system requires, and the system itself.
[In Addition, why does damage detection system has a GetUnitZ function?]

-- Create a default instance of the system
local illusionsInstance = PseudoIllusionsSystem:GetInstance(
    FourCC('API0'),  -- Ability ID to mark illusions
    FourCC('API1'),  -- Spell ID that creates illusions
    "metamorphosis", -- Buff order string
    FourCC('API2'),  -- Buff ID to disable
    60.0,            -- Illusion duration (seconds)
    200              -- Spawn offset distance
)

This would seem more appropriate as an example, and not inherent to the system.

--[[
    Handles item pickup attempts by illusions (prevents them from taking items)
    @param illusionData: The data table for the illusion attempting pickup
--]]
function PseudoIllusionsSystem:OnUnitPickupItem(illusionData)
    if not illusionData or not illusionData.triggeringUnit then return end

    local illusionUnit = illusionData.triggeringUnit
    local item = GetManipulatedItem()
    if not item then return end

    -- Drop the item back where it was picked up
    SetItemPosition(item, GetUnitX(illusionUnit), GetUnitY(illusionUnit))
end

The "illusion" already can't pickup items since it has the ability which locks the inventory, this function is redundant.
You have similar redundant checks:
if not GetEventDamageSource() or not BlzGetEventDamageTarget() then return end
It's not possible for a non-existing unit to be damaged, so the 2nd operand isn't needed.

GetUnitX(target) + self.illusionOffset * Cos(GetRandomReal(0, 6.283)),
    GetUnitY(target) + self.illusionOffset * Sin(GetRandomReal(0, 6.283)),

That number is bj_PI*2 (TAU), you should use it.

It doesn't look like the system accounts for units with Reincarnation ability.
I think you may also have to copy the damage "number of dice" and "sides per die" onto the illusion.
Item position becomes always slot-ordered if the hero happens to have some empty slots while having an item at any slot from 2-5. It's a minor issue. If you're trying to mimic the standard illusions, then it's missing the change of unit vertex colour for allies, besides the owner of illusion. Even with experience disabled I received less exp than I should have. I've got 85 exp without illusions, 43 exp with 1 illusion. Furthermore, the illusions are currently giving exp points to enemies, and bounty if they happen to be killed, and if bounty is enabled.

I think you have a nice idea and the code itself isn't bad, but it needs some reworks in several aspects, such as:
More configurables, such as the special effect paths for the creation and the destruction of the illusions.
A clearer API: PseudoIllusionsSystem.Create(sourceUnit, illusionId, spawnOffset, damagedFactor, attackDamageFactor) -> unit
PseudoIllusionsSystem.IsIllusion(unit) -> boolean
Doesn't need to have these exact names of course.
Is it easier for the user to input the illusion Id version themselves or the system can do it itself? I don't know. However, I wouldn't risk the FourCC and reverse functions you made, specially since blizzard's function may fail in some edge case (Luashine discovered this).

And this was mostly about the Lua version. Next is the vJASS (you have JASS tag instead of vJASS )

Thank you so much for taking the time to review my spell. I'm very new to creating custom spells in WC3 and still learning. I had an idea I wanted to try, but I wasn't sure how to begin, so I've been using Google and AI a lot to help me build it. I'm sure there are many issues with it, but hopefully, as I learn more, I'll be able to fix them all.