Logic making my life terrible.

[grandpaw broon]

Hi all,

I have been here for 2 weeks now attempting a project and im still here after countless restarts and it all comes down to what i thought was initially simple.

To simplify this, consider the Node class is a simple circle sprite with mouse events enabled. A user clicks on a Node instance, moves the mouse and then clicks to create a new Node.

Once a new node is created, a Pair class instance is created. A Pair takes a reference to what node was originally clicked as well as the newly created node. It draws a line between these nodes.

Lets say i have 3 nodes (n) and therefor 2 Pairs(---).


n1 n2 n3
o-----o-----oI also have a pairsArray and a nodesArray containing these objects.

Double click means destroy, so n2 is double clicked. My problem now comes from dealing with the pairs.

This is what i have.


function onNodeDoubleClick(e:MouseEvent)void
{
var node:Node = e.target as Node;

for (var i:int = 0; i < pairsArray.length; i++)
{
var pair:pair = pairsArray[i] as Pair;

if (node == pair.node1 || node == pair.node2)
{
var trash:Pair = pairsArray.splice(pairsArray.indexOf(pair));
trash.destroy();
}
}
}

I have redone this whole class about 10 times and no matter what i do it always comes to these Pairs letting me down. Am i using arrays incorrectly or is it my logic failing me?

Thanks for reading.

[Favardin]

Try this (might contain typos):



function onNodeDoubleClick(e:MouseEvent)void {
var node:Node = e.target as Node;
for (var i:int = 0; i < pairsArray.length; i++) {
var pair:Pair = pairsArray[i] as Pair;

if (node == pair.node1 || node == pair.node2){
pairsArray.splice( i, 1 );
pair.destroy();
i--;
}
}
}


Some notes on your original code:

you forgot the second parameter of splice(..), which denotes how many elements should be deleted from the given index onward
you already know the index of the current pair (i), so no need to call indexOf() (in fact, this might get a performance problem if you have lots of nodes!)
you already have a referenc to the pair you want to destroy (pair), so no need the get if from splice(..) again


Work on your code-fu ;)

[mathew.er]

What's actually wrong, how does the current state differ from expected beaviour?

You don't need to use pairsArray.indexOf(pair), i should already hold the same value.

What environment are you using for development? If it has a debugger, use it :) Also, are you familiar with the trace () function?

If the Node class contains other DisplayObjects that are drawn into, the e.target doesn't have to hold value of the Node instance, but of that DisplayObject instead, try using e.currentTarget to make sure.

Otherwise the code looks ok, there should be an error somewhere else, can you post relevant code of both classes?

[grandpaw broon]

Thanks guys,

I think what is wrong is that i dont get consistant behavoir and end up getting errors.

It comes down to me being confused by Arrays.

pairsArray = [pair1, pair2];

if i happen to destroy a pair
var pair:Pair = pairArray[1];
pair.destroy();
pair = null

Then i thought that pairArray would be the same, pair2 would still be there, only its nulled? So i thought this would cause me problems? It was on this basis i did a splice. I thought if i dont delete the element from the array then it still exists.

Also, how do you actually destroy an object? my Destroy() method removes all event listeners and display objects then i set the pair = null. Does this mean my pair is now destroyed (or set to be destroyed by garbage collector?) and therefor does this process alone remove it from the pairsArray?

My own example is much more complex than this but the node/pair is as simple as i could make it for discussions sake.

PS I found a collection class online today by mistake. Would a collection of Nodes be easier to manage?

[grandpaw broon]

Here is my double click, all you need to do is change Anchors to Nodes to follow on from before.



private function onAnchorDoubleClick(e:AnchorEvent): void
{
var anchor:Anchor = e.dispatcher;
var duds:Array = new Array();

for each (var p:Pairing in pairs)
{
if (p.anchor1 == anchor || p.anchor2 == anchor)
{
duds.push(p);
}
}

for each (var pair:Pairing in duds)
{
destroyPairing(pair);
pairs.splice(pairs.indexOf(pair), 1);
pair = null;
}

destroyAnchor(anchor);
anchor = null;
isDragging = false;
highlight.graphics.clear();

}


You can try this SWF out as its attached.

Notice the inconsistancy, some double clicks delete but some just dont. By the way, this is once again me restarting and i have tried to take into consider the lack of need for the splice method but i dont see how else to remove the nulled element from the array.

[Mentalist]

It may be better to just have each node keep track of which other nodes it's attached to. That way, you just need to call a method of the double-clicked node to have it break all it's links.

[grandpaw broon]

It may be better to just have each node keep track of which other nodes it's attached to. That way, you just need to call a method of the double-clicked node to have it break all it's links.

Thanks for your suggestion Mentalist.

The problem is this is only the input screen for my game. Once a pair is created, it knows who both its anchors are as well as lots of other information, its min/max length, its weight its cost and so on.

These anchors and pairs will become physics based entities later in the game.

I just dont understand why its being so troublesom, i dont understand why the logic fails because all i am trying to do is say "check over all pairs and destroy any who reference this double clicked on anchor" but like i say, no matter what i try, i always get this buggy behaviour.

It almost looks as though only 1 side of a pair is checked, thats why some seem to destory as required but some dont.

[grandpaw broon]

I just wanted to bump this for the final time because im still stuck on this.

EDIT: SOLVED

I dont believe it, you know what it was?

Because i created a pair but didnt add it to the display list (for validation, so no dupes exist) when i called destroyPair(pair) it was trying to remove a pair from the display list which didnt exist so threw an error.

Likewise on my previous attemps like the attachment above, the opposite happened. Because i didnt add the validation pair to the pairsArray but did add it to the display list, it was skipping the removal of the pair from the display list which meant i had 3 display objects yet only 2 pairs. That is why eventually an error was thrown.

So this simple overlook was essentially the basis of over 30 hours of failure.

Thanks for the tip with regards to the debugger! I think i am best friends with that bad boy now.