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

[threephi]

Actually, while testing and exploring the workaround, I discovered that IsBarren does not depend on who owns the asteroid, in other words, it will return true for any unplanted asteroid, not just team 0.  GetNumTrees is also deficient as it only counts trees "above ground" and thus cannot distinguish between a virgin, ie completely unplanted, asteroid, and one whose trees all have their roots exposed.  IsBarren is the only function I could find that makes that distinction, and it is an important one.

A virgin unplanted asteroid is fair game for any faction with enough seedlings present, no matter who owns it.  First root to the center takes the asteroid.  FWIW I do not think the game devs considered the possibility that barren asteroids would be assigned non-grey team owners so the function description is incorrect but it is much more useful the way it is.

[threephi]

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?

No.  It returns true if there are no roots touching the center.  That might be the most succinct way to describe the function.  This is part of the special usefulness of IsBarren, it allows multiple factions to attempt to plant a virgin asteroid at the same time (so long as there are enough tree slots and each has enough seedlings present, naturally) and start the roots racing to the center.

The fact that IsBarren is false on a planted asteroid whose roots touch the center is very important for the case where the asteroid has only one tree slot with roots which have been exposed and thus are open for infection.  In that situation, the owner faction can immediately plant, but all other factions must infect in order to take the asteroid.  It is noteworthy that in this case, FreeSlots() returns 1, AvailableStructureSpace() returns TRUE, and GetNumTrees() returns 0.  So if not for IsBarren() being false and the owner being a different faction (the absence of those tests are precisely the shortcoming in PlantDysonTree and PlantDefenseTree), this asteroid would appear as a virgin asteroid and permit non-owner factions to plant immediately.  FWIW, when working up the fix, I looked for a function that directly indicated whether an asteroid had a tree with exposed roots and couldn't find one.  When I worked up a few test levels using the pre-corrected infected AI code when I was exploring this bug, I also noted that opposing factions with the proper number of seedlings (enough to plant but not enough to evaluate as an attack) on such an asteroid planted immediately, without infecting the roots first.