REMOVED

Started by Cyndanera, June 20, 2018, 03:03:59

Previous topic - Next topic

Cyndanera

REMOVED

Matty

It's hard to be precise but is it possible that add_data is local and is not actually reaching '2' - because it goes to '1' then goes out of scope when the function is exited?

Cyndanera

#2
REMOVED

Matty

Yes...but does it actually reach '2'?

If it is going out of scope before it is added to again it will revert to 0 and only alternate between 0 and 1.

If it is global then it will continue to add each time you enter that function correctly (when you click) and go to 3 and so on.

But I'm guessing it is going out of scope.

Cyndanera

#4
REMOVED

Derron

I haven't checked your code but my eyes were glued to this:

Code (BlitzMax) Select

For t:tile = EachIn tile_list
If add_data = 2 And MouseX()/tsize = t.x And MouseY()/tsize = t.y Then DestoryTile(t)
Next
If add_data = 2 Then
Select tiletype
Case 1
CreateTile(1, MouseX()/tsize, MouseY()/tsize)
End Select
EndIf
EndIf


Does "DestoryTile" (sic!) alter "add_data"? If not, you could save some comparisons:

Code (BlitzMax) Select

If add_data = 2
  local mouseTileCol:int = MouseX() / tsize
  local mouseTileRow:int = MouseY() / tsize
  For t:tile = EachIn tile_list
    If mouseTileCol = t.x and mouseTileRow = t.y Then DestoryTile(t)
  Next

  Select tiletype
    Case 1
      CreateTile(1, mouseTileCol, mouseTileRow)
   End Select
EndIf


And even this can have issues: you remove a tile in "DestoryTile" so the "eachIn"-iterator might create trouble (skipping items in the for-loop). Do avoid concurrent list modifications when compiling with "vanilla" (NG and maxmods/brl.mod might have some checks for this issue).
Others do not do it and seem to not have trouble but in my game TVTower we experienced issues with concurrent list modifications which is why Brucey added some stuff to the linkedlist (TList)-module.

Aside of that: what did my optimization do:
- only check "add_data" once
- avoid calculating "col/row" for each tile in the list and also for the tile-creation call by using a calculated-once local variable.


There is one part which could get optimized a bit more:
But this only works if there is only one tile per "grid cell" (col/row)
Code (BlitzMax) Select

  For t:tile = EachIn tile_list
    If mouseTileCol = t.x and mouseTileRow = t.y Then DestoryTile(t)
  Next

'becomes

  For t:tile = EachIn tile_list
    If mouseTileCol = t.x and mouseTileRow = t.y
      DestoryTile(t)
      exit 'no need to check other tiles, we already found one
    EndIf
  Next


bye
Ron

Cyndanera

#6
REMOVED

Derron

You know the problem so you might als know the solution.

if you do not want a variable to increment, only increment in the cases in which it should. So here: only increase if there is a tile.


bye
Ron