Bug: PlantDysonTree and PlantDefenseTree allow illegal planting

[threephi]

Hi guys, I have identified a bug in two game functions, PlantDysonTree and PlantDefenseTree.  They will both plant trees belonging to the specified faction on any asteroid, regardless of existing trees or who owns the asteroid, so long as there are enough seedlings and at least one open tree slot.

The bug can be seen in action in the following very simple test level (also attached as a file to this post):

Code: [Select]

function LevelSetup()
Globals.G.Asteroids=(0)
Globals.G.EnemyFactionsMin=(0)
Globals.G.EnemyFactionsMax=(0)
Globals.G.GreysProbability=(0)
Globals.AI.GraceTimer=(99999)

a = AddAsteroidWithAttribs(0,0,0.5,0.5,0.5)
a.owner = 1
a.TreeCap = 5
a:SetRadius(500)
a.SendDistance = 5000
a.Moveable = false
a:Reveal(1)
a:AddSeedlings(10,2,0.3,0.3,0.3)
a:AddSeedlings(10,3,0.3,0.3,0.3)

t = a:AddDysonTree()
t:LevelUp()
t:LevelUp()
t:LevelUp()
end

function LevelLogic()
a = GetAsteroid(0)
a:PlantDysonTree(2)
a:PlantDefenseTree(3)

while GameRunning() do
coroutine.yield()
end
end
The result is the following:



As can be seen in this screen cap, PlantDysonTree and PlantDefenseTree added trees when they shouldn't have been able to do so.

I discovered this bug while playing one of Annikk's levels, EXTREME PWN LAZ0RZ!!12, before it was updated with a fix to correct for the illegal planting.  Because of that level's size, the time it takes to complete, and several nuances of the infected AI code, this bug occurred frequently.

[Aino]

Well, why bother so much of this? And this actually allows more stuff to be done, to prevent it; just check if the owner is themself else that there is noone that have been planting one it...

[annikk.exe]

Well, why bother so much of this? And this actually allows more stuff to be done, to prevent it; just check if the owner is themself else that there is noone that have been planting one it...

That alone won't work, two different empires can plant on an asteroid at the same time.  The asteroid doesn't actually change owners until the roots reach the center.

[Aino]

Ok, but thats very odd happen, so why not make a variable: planted(array for all roids) which turns from not true to true once an emprie have planted a tree there, then because pf that, only one empire can plant there... Now you can check if the tree is alive before planting. For players, well it will be checked once the asteroid is checked. I don't have this problem with my AI after I did this...

[Aino]

But there could be checks for this fucntion and a new one will be put in: ForcePlantTree, which will force the empire to plant a tree and occupate the asteroid... useful in special events

[threephi]

The workaround code is rather simple, but I'm pointing it out because it's clearly not how it's supposed to work.  For one thing, it's contrary to the function description given in the lua reference (although that description does contain a subtle error discussed below).  You're not supposed to have asteroids with trees belonging to two (or more) different factions, and when it happens it can really mess things up.

Given a = asteroid in question; f = faction whose tree you want to plant:

Code: [Select]

if a:AvailableStructureSpace() and (a.IsBarren() or a.owner == f) then
        PlantDysonTree(f)
end
In looking into this bug I did gain an appreciation of the subtleties involved in planting a tree so it's not so straightforward as a simple owner check.

Well, why bother so much of this? And this actually allows more stuff to be done, to prevent it; just check if the owner is themself else that there is noone that have been planting one it...

That alone won't work, two different empires can plant on an asteroid at the same time.  The asteroid doesn't actually change owners until the roots reach the center.

Exactly.  You can have legitimate situations where two different factions can legally plant a tree at roughly the same time. If you have a virgin asteroid with zero existing trees (and room for trees of course), the new owner will be whoever gets their roots to touch the center first, in which case all the trees will become theirs.  Once there is one root system that touches the center, the rules are different.

Here's the subtle omission in the function description I mentioned above:

   Attempt to plant a Dyson tree using the seedlings present at the asteroid; if nothing can be done (all tree slots are taken, or there are not enough seedlings or the asteroi [sic] is owned by some other faction than team 0 or the given faction) the function does nothing and returns nil. Otherwise the function returns a Structure, which is the base type of the trees in Dyson.

Any faction can plant on any asteroid with zero existing tree roots so long as there are tree slots, no matter which faction is designated as the owner.

[Aino]

Mhm...

A tree/owner/root check will do the trick, cause if a tree is planted, it will immediatly be added to the amount of trees on the roid. Just tested to plant a tree, just after the tree was PLANTED, it was already counted as a tree on the asteroid.

So if you're using a custom AI, just add a root array and make it change to true for the asteroid if there is an existing tree on the roid and the root array is false for the roid. then if it is false it'll be allowed to plant

[annikk.exe]

You don't need an array to check for it.

[Aino]

Maybe not, but thats how I did it

[threephi]

That's kind of exactly what AvailableStructureSpace() does... true if there is room to plant a tree, false if not.

[Aino]

IsBarren checks it too I guess

[Pilchard123]

No. What IsBarren() does is check if the roid has NO trees planted and belongs to the greys. It could return true but you would not be able to plant a tree there if the treecap was 0. AvailableStructureSpace() will return true if treecap-plantedtrees > 0.

FreeSlots() tells you how many trees can be planted there.

[annikk.exe]

Is it correct that IsBarren() will return false if a tree has been planted but its roots have not yet touched the center, and thus the asteroid owner is still 0?

[Aino]

I would actually understand that better if you wrote it like a code xD

But I haven't tested much about IsBarren(), but this problem is made out of the oddest occasion you can get into. Because I've never in my gametime seen two empires having over 10 seedlings and the asteroid is currently having 0 trees...

New suggestion: Add IsRooted field(would be best, but if you can't, do a checking function)...

If it is a field, then if the editor sets it to false on purpose, it will be unrooted at once, leaving no roots and 0 trees.

[Pilchard123]

Okay...

IsBarren() is the same as.....

function blah(Asteroid)
  barren = false
  if Asteroid.owner == 0 and Asteroid:GetNumTrees() == 0 then
    barren = true
  end
  return barren
end