OpenClonk Bugtracker - OpenClonk
View Issue Details
0002048OpenClonkEngine - C4Scriptpublic2018-12-11 13:022019-06-23 18:27
ReporterMarky 
Assigned ToMarky 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version 
Target VersionFixed in Version9.0 
Summary0002048: Find_ID() does ignore objects under unknown circumstances.
DescriptionThe following script caused the problem - the object is not in a separate layer or something:

public func Get(object to)
{
    AssertDefinitionContext();

    // Works only the second time the player aims, which is really weird...
    // the cursor object exists, and can be found via Find_ActionTarget()
    // but Find_ID ignores it
    //
    // return FindObject(Find_ID(this), Find_ActionTarget(to));
    for (var attached in FindObjects(Find_ActionTarget(to)))
    {
        if (attached->GetID() == this)
        {
            return attached;
        }
    }
    return nil;
}

TagsNo tags attached.
Attached Files

Notes
(0006233)
Foaly   
2019-04-29 08:40   
I can't reproduce this.
Do you have a full test case?
(0006235)
Clonkonaut   
2019-04-29 22:11   
Shouldn't Find_ID(this) be Find_ID(this->GetID())?
(0006240)
Foaly   
2019-04-30 06:31   
As far as I can tell it is called within the definition, for example:
Ore->Get(whatever);

In my tests, both versions return the same object, so the above code works.

But there is probably a special case, where Find_ID fails, but I can't reproduce it.
(0006242)
Clonkonaut   
2019-04-30 07:38   
And 'this' in a definition call is 100% equivalent to an ID?
(0006243)
Foaly   
2019-04-30 07:44   
Well, I'd expect Find_ID(x) to find all objects where GetID() == x.
No matter what 'x' actually is.

If there are special cases, we should at least note it in the documentation.
(0006245)
Clonkonaut   
2019-04-30 08:25   
My question is not about how Find_ID() finds objects but if the code in the example is correct.

Meaning if Find_ID(this) is equivalent to Find_ID(this->GetID()) when used in a definition call or if you have to use this->GetID() regardless of whether or not this a definition or object call. Meaning that 'this' is never of the type ID.
(0006246)
Foaly   
2019-04-30 09:16   
Yes, but in the example, the

attached->GetID() == this

is true.
(0006248)
Marky   
2019-05-01 11:16   
So, I had a look at the code in order to solve it:

This function is called to check whether the object is found with Find_ID(def):

bool C4FindObjectDef::Check(C4Object *pObj)
{
    return pObj->GetPrototype() == def;
}

And this is what GetID() does:

static C4PropList* FnGetID(C4Object *Obj)
{
    // Return id of object
    return Obj->GetPrototype();
}

Finally, using this->GetID() is not viable (I'd assume that it is the same when you call that from an ID from within its script):

ERROR: GetID: must be called from object context
 by: Scenario->Scenario
-> Scenario::Rock->GetID()

I'll try the code in question again, this time with log output (:O)
(0006249)
Marky   
2019-05-01 11:52   
At first I thought that C4FindObject::Find and C4FindObject::FindMany seem to make the difference, but here is some more log output that helps:

// Various log outputs from creating the object
AddTo: Object(95), Be Object(8)
Added new cursor Object(95)
// Cyclic output for finding objects
Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => nil / []
Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => nil / []
Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => nil / []
Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => nil / []
Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => nil / []
Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => nil / []
// Then decided to do a small investigation
-> Scenario::Object(95)
 = Object(95)
-> Scenario::FindObjects(Find_ID(CMC_Virtual_Cursor))
 = []
-> Scenario::FindObjects(Find_ActionTarget(Object(8)))
 = [Object(95)]
-> Scenario::Object(95)->GetID()
 = CMC_Virtual_Cursor

// So the object is still there and has the right ID, but it is not found by Find_ID
-> Scenario::FindObjects(Find_ID(CMC_Virtual_Cursor), Find_AnyLayer())
 = [] // Still not there? Weird!
-> Scenario::Object(95).Visibility
 = 2 // OK, we have VIS_Owner, maybe that was the cause?
Scenario::Object(95).Visibility = VIS_All
 = 0
-> Scenario::FindObjects(Find_ID(CMC_Virtual_Cursor))
 = []
-> Scenario::FindObjects(Find_ID(Object(95)->GetID()))
 = []
// Then (game still paused), only after I duplicate the object it is found:
-> Scenario::FindObjects(Find_ID(CMC_Virtual_Cursor))
 = [Object(99), Object(95)]

Maybe this is a weird problem in the object list?
(0006250)
Marky   
2019-05-01 12:01   
Updated the log output a bit:

Using the live code from the above example:
// First aiming
[13:54:31] AddTo: Object(96), Be Object(8)
[13:54:31] Added new cursor Object(96)
[13:54:31] Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => nil / [] of Find_ID [], Find_ActionTarget [Object(96)]

Now with the dead code again:
// First aiming
[13:57:23] AddTo: Object(95), Be Object(8)
[13:57:23] Added new cursor Object(95)
[13:57:23] Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => nil / [] of Find_ID [], Find_ActionTarget [Object(95)]
// Second aiming
[13:57:25] AddTo: Object(104), Be Object(8)
[13:57:25] Added new cursor Object(104)
[13:57:25] Get: [10, CMC_Virtual_Cursor], [14, Object(8), 0] => Object(104) / [Object(104), Object(95)] of Find_ID [Object(104), Object(95)], Find_ActionTarget [Object(104), Object(95)]
(0006251)
Foaly   
2019-05-02 07:08   
Since after duplicating the object it is found, maybe it's a problem with this function:

bool C4FindObjectDef::IsImpossible()
{
    return !def || !def->GetDef() || !def->GetDef()->Count;
}

Probably the count is kept incorrectly, and duplicating the object increases it.
Since you are able to reproduce it, you could check that.
(0006252)
Marky   
2019-05-02 16:45   
Can confirm. I removed the last condition and it gets found the first time. Now let's see where the count is updated incorrectly...
(0006253)
Marky   
2019-05-02 17:37   
Ok, after more debugging I found out that this scenario has, somehow, a setup where AssignRemoval is called twice and the definition count decreases from 1 to 0 to -1. Therefore, when the new object is created, we get a count of 0...
(0006254)
Marky   
2019-05-02 17:59   
(Last edited: 2019-05-02 18:21)
Just an update:
The part that causes the problem is:

func AttachTargetLost()
{
   Log("Attach target lost");
   RemoveObject();
   Log("Attach target removal done");
   return;
}

With additional log output in the engine (next local build with stack trace):

[19:49:16] Attach target lost
[19:49:16] Decrease Def->Count of .../Cursors.ocd (AssignRemoval) Object 16: 0
[19:49:16] Attach target removal done
[19:49:16] Decrease Def->Count of .../Cursors.ocd (AssignRemoval) Object 16: -1

Update:

Reproducing it is easy
1) Attach object A (with RemoveObject() in AttachTargetLost()) to object B
2) Put object B inside object C
3) Remove object C with RemoveObject()

The removal of object C triggers the removal of object A. When the object is removed, apparently it loses its attach target (from my log history). Then it gets removed again, but its status is not 0 yet (I assume), so it can be removed twice.

Unrelated:
The engine calls AttachTargetLost by itself, but there are also a lot of objects (but not all) that additionally define AttachTargetLost as the AbortCall of an action with DFA_Attach (as in my case, which may be caused by the action going to idle when an object is removed).
Will try out what happens when I remove the abort call.

(0006255)
Marky   
2019-05-02 18:27   
"Duplicate" definition of AttachTargetLost as AbortCall in an action caused the problem.

This happens at 9 instances in Objects.ocd, and it is a problem in any AbortCall that removed the object (which is hard to find out?)...
(0006256)
Marky   
2019-05-02 19:47   
https://github.com/openclonk/openclonk/pull/108

Issue History
2018-12-11 13:02MarkyNew Issue
2019-04-29 08:40FoalyNote Added: 0006233
2019-04-29 22:11ClonkonautNote Added: 0006235
2019-04-30 06:31FoalyNote Added: 0006240
2019-04-30 07:23ClonkonautNote Added: 0006241
2019-04-30 07:24ClonkonautNote Deleted: 0006241
2019-04-30 07:38ClonkonautNote Added: 0006242
2019-04-30 07:44FoalyNote Added: 0006243
2019-04-30 08:25ClonkonautNote Added: 0006245
2019-04-30 09:16FoalyNote Added: 0006246
2019-05-01 11:16MarkyNote Added: 0006248
2019-05-01 11:52MarkyNote Added: 0006249
2019-05-01 12:01MarkyNote Added: 0006250
2019-05-02 07:08FoalyNote Added: 0006251
2019-05-02 16:45MarkyNote Added: 0006252
2019-05-02 17:37MarkyNote Added: 0006253
2019-05-02 17:59MarkyNote Added: 0006254
2019-05-02 18:17MarkyNote Edited: 0006254bug_revision_view_page.php?bugnote_id=6254#r1391
2019-05-02 18:17MarkyNote Edited: 0006254bug_revision_view_page.php?bugnote_id=6254#r1392
2019-05-02 18:21MarkyNote Edited: 0006254bug_revision_view_page.php?bugnote_id=6254#r1393
2019-05-02 18:27MarkyNote Added: 0006255
2019-05-02 18:58MarkyAssigned To => Marky
2019-05-02 18:58MarkyStatusnew => assigned
2019-05-02 19:47MarkyNote Added: 0006256
2019-06-23 18:27MarkyStatusassigned => resolved
2019-06-23 18:27MarkyResolutionopen => fixed
2019-06-23 18:27MarkyFixed in Version => 9.0