koobas.hobune.stream
Problem with nested for loops
Hi everyone,
I nested a for loop inside a for loop but I am not getting the expected behaviour. Basically, I would like every LegalBlackMove objects to get a score of -1000 for every LegalWhiteMove objects but it's not working well.
Let's say that for example we have 24 LegalWhiteMove objects for every LegalBlackMove objects. I would expect the function to return a score of -24000 on every LegalBlackMove objects but instead I am getting -24000 for the first one, -48000 for the second one, -72000 for the third,... and so on.
Here is a simplified version of my code :
function ScoreBlackMoves () {
ListAllBlackLegal(); // this function generates LegalBlackMoves objects
var firstMoves = GameObject.FindGameObjectsWithTag ("LegalBlackMove");
for (var firstMove in firstMoves) {
ListAllWhiteLegal(); // this function generates LegalWhiteMoves objects
var secondMoves = GameObject.FindGameObjectsWithTag ("LegalWhiteMove");
for (var secondMove in secondMoves) {
firstMove.GetComponent(LegalMove).score += -1000;
Destroy(secondMove);
}
}
}
Can anyone please help?
this could be perhaps incredibly overcomplicated
just use something like yourList.Count to get the number involved, in each case.
I believe you get some sort of a list from the ListAllWhiteLegal()? Make sure you clean that list at the start of a new loop. Seems to me you keep stacking the amount of LegalWhiteMoves.
So at the start of the first loop clear the holder:
List.Clear() or if it is a property LegalWhiteMoves = 0;
It's pretty hard to tell without knowing what's going on in ListAllWhiteLegal();
But looking at your script I guess that function is actually instancing new GameObjects. Looking at your inner loop, you are trying to destroy each GameObject that you instanced this way. There are two severe problems to your approach:
Instancing GameObjects just for the scoring, without doing anything to them, is extremely expensive. You should avoid that at (almost) all costs, especially when developing for any Mobile platform, or if the average number of "legal moves" is more than, say 5 each. For example, as @Esa suggested, just use a List instead, or create your own class, or try to store your information somewhere else. Don't create complete GameObjects if you don't need any of the Unity features for them (Update loops, Inspector integration, raycasting/collisions, ...)
Your command
Destroy(secondMove);will do nothing in your loop, the GameObject will stay alive until the end of the current frame. Please take a look at the docs forDestroy. This is the reason the amount of GameObjects tagged "LegalWhiteMove" keeps increasing while processing your loop.You could use
DestroyImmediate, but this solution is much worse than not creating unnecessary GameObjects in the first place.
Thanks a lot Wolfram & Esa,
DestroyImmediate fixes the problem perfectly. I understand using gameObjects may not be the optimal solution for what I am trying to achieve. On the other hand, the function will not be called very often (once every $$anonymous$$utes or so) and is used in a game that is not requiring a lot of ressources (its in the AI component of some kind of chess game).
I am currently learning to code. I would love to use something else than gameObjects to achieve my goal but I am wondering, since I need to create "objects" at runtime that have actual rendered complex gameObjects as variables. Is there any other way? I don't see how I could use anything else than a gameobject to attach a script and do what I need,
Here is the code for ListAllWhiteLegal();
function ListAllWhiteLegal () {
var whitePieces = GameObject.FindGameObjectsWithTag ("WhitePiece"); for (var whitePiece in whitePieces) {
if (whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave1") &&
whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave2") &&
whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave3") &&
whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave4") &&
whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave5") &&
whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave6") &&
whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave7") &&
whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave8") &&
whitePiece.GetComponent(Piece).isLocatedAt != gameObject.Find("whiteGrave9") ) {
gameObject.Find("Center").GetComponent(Intersection).ResetLegal();
gameObject.Find("Board").GetComponent(Selector).selected = whitePiece;
whitePiece.GetComponent(Piece).LegalCheck$$anonymous$$night();
whitePiece.GetComponent(Piece).LegalCheckNormal();
whitePiece.GetComponent(Piece).LegalCheckCenter();
var intersections2 = GameObject.FindGameObjectsWithTag ("Intersection");
for (var intersection2 in intersections2) {
if (intersection2.GetComponent(Intersection).legal == true) {
var go2 = new GameObject("legalWhite$$anonymous$$ove");
go2.tag = ("LegalWhite$$anonymous$$ove");
var l$$anonymous$$ove2 = go2.AddComponent(Legal$$anonymous$$ove);
l$$anonymous$$ove2.selectedPiece = whitePiece;
l$$anonymous$$ove2.previousIntersection = whitePiece.GetComponent(Piece).isLocatedAt;
l$$anonymous$$ove2.nextIntersection = intersection2;
}
}
}
}
}
Well, if you want to visualize the possible moves, you'd of course need GameObjects for them. But if it's solely for calculating a score, or finding the next AI move or something, then a simple, internal, non-Unity data structure class might be better suited.
On the other hand, if this function isn't called that often, it might not be worth to change everything - mostly depends on your target platform, and whether you have a performance problem there.
One other thing, which is probably equally expensive as instancing GameObejcts: go.Find() and go.FindGameObjectsWithTag(). These are quite expensive, dumb search functions that need to traverse the whole scenegraph. If the objects you expect to find don't change, do the Find in Start() or Awake(), and store the result. If the objects do change, you might consider using a pool, or have newly created objects register themselves with your manager, ins$$anonymous$$d of having your manager search for them all the time. But again - maybe not worth the trouble, if you don't have a performance problem., but still something to consider.
Thanks a lot,
I am targeting many platforms so I will try to optimize it as much as possible. I will do the finds in the Start() function.
