Begging for help with simple loop.

BlitzMax Forums/BlitzMax Programming/Begging for help with simple loop.

Reda Borchardt(Posted 2009) [#1]
Please help me. I am so frustrated that I could shout at my screen.

I am simply trying to produce a list of unique resolutions with no duplicates whatsoever.

I have been playing around with this the entire night and I still end up with duplicates. I don't understand why!!!

The second loop is entirely redundant. However, I am getting so desperate that I tried to break it down into two separate loops to isolate the issue.

Your help would be greatly appreciated.

The function to look at specifically is:
Function PopulateRes()
		Local m:Res
		Local check:String[CountList(resolution.resolutionlist)]
		Local i:Int = 0
		Local j:Int = 0
	      Local found:Int	

		For m = EachIn Resolution.Resolutionlist
			check[i] = m.resolution
			i=i+1
		Next
		
		For j=0 To check.length-1
			found = 0
			For m = EachIn Resolution.Resolutionlist		
				If m.resolution = check[j]										      			
					If found > 1 Then check[j]=""						
					found = found + 1 
				End If				
			Next			
		Next

		For i=0 To check.length-1
		  If check[i]<>"" 
			AddGadgetItem rescombobox, check[i]
			Print check[i] 'Testing
		  EndIf
		Next	
	End Function


Here is the full code

' <HEADER>
	SuperStrict
	Import maxgui.drivers
	Import brl.Graphics	
	Global APP_WIDTH:Int = 640
	Global APP_HEIGHT:Int = 240
' </HEADER>

' <OBJECTS>
	Type Resolution
		Global ResolutionList:TList = New TList
		Method New()
			ResolutionList.AddLast(Self)
		End Method
	End Type

	Type Res Extends Resolution
		Field Resolution:String
		Field Depth:String
		Field Hz:String	
		
		Function Create:Res(r:String,d:Int,h:Int)
			Local ent:Res = New Res
			ent.Resolution = r
			ent.Depth = d
			ent.Hz = h
			Return ent
		End Function
	End Type
' </OBJECTS>

' <FUNCTIONS>
	Function PopulateRes()
		Local m:Res
		Local check:String[CountList(resolution.resolutionlist)]
		Local i:Int = 0
		Local j:Int = 0
	      Local found:Int	

		For m = EachIn Resolution.Resolutionlist
			check[i] = m.resolution
			i=i+1
		Next
		
		For j=0 To check.length-1
			found = 0
			For m = EachIn Resolution.Resolutionlist		
				If m.resolution = check[j]										      			
					If found > 1 Then check[j]=""						
					found = found + 1 
				End If				
			Next			
		Next

		For i=0 To check.length-1
		  If check[i]<>"" 
			AddGadgetItem rescombobox, check[i]
			Print check[i]
		  EndIf
		Next	
	End Function
' </FUNCTION>

' <GUI>
	Local wx:Int = (GadgetWidth(Desktop()) - APP_WIDTH) / 2
	Local wy:Int = (GadgetHeight(Desktop()) - APP_HEIGHT) / 2
	Global window:TGadget = CreateWindow("Launcher", wx, wy, APP_WIDTH, APP_HEIGHT, Null, WINDOW_TITLEBAR | WINDOW_CLIENTCOORDS)
	Global rescombobox:TGadget = CreateComboBox(4, 50, 200, 22, window)
	Global depthcombobox:TGadget = CreateComboBox(4, 80, 200, 22, window)
	Global refreshcombobox:TGadget = CreateComboBox(4, 110, 200, 22, window)
' </GUI>

' <LISTS>
	Local mode:TGraphicsMode
	For mode:TGraphicsMode = EachIn GraphicsModes()
	    Local r:Res = res.Create(mode.width + " X " + mode.height,mode.depth,mode.hertz)
	Next
' </LISTS>
	
' <TEST>
	PopulateRes()
	'Local m:Res
	'For m = EachIn Resolution.Resolutionlist
      '   Print m.resolution
	'Next
' </TEST>

' <BODY>
	While WaitEvent()
	   Select EventID()
	
	   	Case EVENT_WINDOWCLOSE
	   		End
	
	   	Case EVENT_APPTERMINATE
	   		End
	   End Select
	Wend
' </BODY>




Grey Alien(Posted 2009) [#2]
If found > 1
This should be >=1


Reda Borchardt(Posted 2009) [#3]
Unless my compiler is buggy, this doesn't work for me. Does it work for you?


_Skully(Posted 2009) [#4]
I dont have Max yet so excuse me if I'm daft...

Where do you eliminate duplicates?

	Local mode:TGraphicsMode
	For mode:TGraphicsMode = EachIn GraphicsModes()
	    Local r:Res = res.Create(mode.width + " X " + mode.height,mode.depth,mode.hertz)
	Next


In that loop you should put something like

	Local mode:TGraphicsMode
	For mode:TGraphicsMode = EachIn GraphicsModes()
		if ResNotRegistered(r:Res) 
		    Local r:Res = res.Create(mode.width + " X " + mode.height,mode.depth,mode.hertz)
		end if
	Next



of course you would need to create the ResNotRegistered function that just checks the current list of Resolutions against the incoming resolution


Zeke(Posted 2009) [#5]
		For j=0 To check.length-1
			found = 0
			For Local jj:Int = j To check.length-1
				If check[jj] = check[j]
					found = found + 1 								      			
					If found > 1 Then check[jj]=""				
				End If				
			Next			
		Next



_Skully(Posted 2009) [#6]
Oops..

	Local mode:TGraphicsMode
	For mode:TGraphicsMode = EachIn GraphicsModes()
		local rmode=mode.width + " X " + mode.height
		if ResNotRegistered(rmode) 
		    Local r:Res = res.Create(rmode,mode.depth,mode.hertz)
		end if
	Next



Reda Borchardt(Posted 2009) [#7]
Skully: Thanks for your help. I also can't get that to work. I still get duplicates on a couple of resolutions. Whatever and whichever way I try to write a function that returns True or False, I always end up with a few false negative. (Especially with the resolution 320 x 200)

Zeke: I can't wait to try your version out. I cannot see in which way it is different to what I currently have. I tried it using two arrays the first time around and removing items from the first list.
I will give it a go as soon as I am back in front of my screen. (Did you test it yourself?)


Muttley(Posted 2009) [#8]
Here's how I get a de-duplicated list of available graphics modes in the retroremakes framework:

	' Find graphics modes for all available drivers, de-duplicate
	' and exclude modes that don't appear on both drivers (where
	' available)
	Method FindGraphicsModes()
		'Get the OpenGL Modes first	
		SetGraphicsDriver(GLMax2DDriver())
		graphicsModes_ = ListFromArray(GraphicsModes())
		
		TGameEngine.GetInstance().LogInfo("OpenGL Graphics Modes Found: " + graphicsModes_.Count())
		'DirectX modes if on Windows
		?win32
			SetGraphicsDriver(D3D7Max2DDriver())
			Local dxModes:TList = ListFromArray(GraphicsModes())

			SetGraphicsDriver(D3D9Max2DDriver())
			Local dx9Modes:TList = ListFromArray(GraphicsModes())
			
			For Local mode:TGraphicsMode = EachIn dx9Modes
				dxModes.AddLast(mode)
			Next
			
			TGameEngine.GetInstance().LogInfo("DirectX Graphics Modes Found: " + dxModes.Count())
			'Remove DirectX Modes that aren't available under OpenGL
			
			For Local findMode:TGraphicsMode = EachIn dxModes
				Local found:Int = False
				For Local mode:TGraphicsMode = EachIn graphicsModes_
					If findMode.width = mode.width And ..
						findMode.height = mode.height And ..
						findMode.depth = mode.depth And ..
						findMode.hertz = mode.hertz
							found = True
							Exit
					End If
				Next
				
				If Not found
					dxModes.Remove(findMode)
				End If
			Next
			
			'Remove OpenGL Modes that aren't available under DirectX
			For Local findMode:TGraphicsMode = EachIn graphicsModes_
				Local found:Int = False
				For Local mode:TGraphicsMode = EachIn dxModes
					If findMode.width = mode.width And ..
						findMode.height = mode.height And ..
						findMode.depth = mode.depth And ..
						findMode.hertz = mode.hertz
							found = True
							Exit
					End If
				Next
				
				If Not found
					graphicsModes_.Remove(findMode)
				End If
			Next
			
			'Merge the lists
			For Local mode:TGraphicsMode = EachIn dxModes
				graphicsModes_.AddLast(mode)
			Next
		?
		
		' Now sort and deduplicate
		graphicsModes_.Sort(True, TGraphicsService.GraphicsModeSort)
		TGameEngine.GetInstance().LogInfo("Total Graphics Modes Found: " + graphicsModes_.Count())
		graphicsModes_ = DeDuplicateGraphicsModes(graphicsModes_)
		TGameEngine.GetInstance().LogInfo("Final De-Duplicated Graphics Modes Found: " + graphicsModes_.Count())
	End Method
	
	Function GraphicsModeSort:Int(o1:Object, o2:Object)
		Local o1mode:TGraphicsMode = TGraphicsMode(o1)
		Local o2mode:TGraphicsMode = TGraphicsMode(o2)

		Local compare:Int

		If o1mode.width < o2mode.width
			compare = -1
		ElseIf o1mode.width > o2mode.width
			compare = 1
		Else
			If o1mode.height < o2mode.height
				compare = -1
			ElseIf o1mode.height > o2mode.height
				compare = 1
			Else
				If o1mode.depth < o2mode.depth
					compare = -1
				ElseIf o1mode.depth > o2mode.depth
					compare = 1
				Else
					If o1mode.hertz < o2mode.hertz
						compare = -1
					ElseIf o1mode.hertz > o2mode.hertz
						compare = 1
					Else
						compare = 0
					End If
				End If
			End If
		End If
		
		Return compare
	End Function
	
	
	Method DeDuplicateGraphicsModes:TList(modes:TList)
		Local deDupedModes:TList = New TList
		Local first:Int = True
		For Local mode:TGraphicsMode = EachIn modes
			If first
				deDupedModes.AddLast(mode)
				first = False
			Else
				If GraphicsModeSort(deDupedModes.Last(), mode) <> 0
					deDupedModes.AddLast(mode)
				End If
			End If
		Next
		Return deDupedModes
	End Method


There's a link to the framework in my sig if you want to check out the code in-situ. It's contained in the TGraphicsService.bmx file which is one of the core services of the framework.

Muttley


grable(Posted 2009) [#9]
How about using a TMap? it sorts at insertion time and does not allow duplicates. Throw in a decent compare method, or just use strings...


Reda Borchardt(Posted 2009) [#10]
Dear grable,

That looks really useful. Thanks.
I would still like to figure out what is wrong with my code. I cannot get my head around what is causing the issue. It should be so simple!


Pete Rigz(Posted 2009) [#11]
The reason it's not working is because you have your loops round the wrong way. You should iterate through the ResolutionList first and then the array. Each time it found a dupe, "j" had not increased in value so it was nulling the same string in the array each time it found one. Also, I think it should increment found before the if statement, not after.

It should look like this:

		For m = EachIn Resolution.Resolutionlist	
			found=0
			For j=0 To check.length-1	
				If m.resolution = check[j]								
					found = found + 1		      			
					If found > 1 
						check[j]=""						 
					End If
				End If		
			Next		
		Next	


Like Grable says, Tmap would be a good approach to this. Hope that helps!


Reda Borchardt(Posted 2009) [#12]
Awesome!! Can't wait to try it out and also learn about Tmap. Thanks.


Reda Borchardt(Posted 2009) [#13]
Thank you very much.

I learned something new today.

   local i:int
   for i=0 to 10
      print i 'result is 1 to 10
   next
   print i 'result is 11 !! (Now that is interesting)


If I print "i" after the loop, the value is 11.


Mahan(Posted 2009) [#14]
I'll recommend this idiom (used in Java/BlitzMax etc) to correctly modularize code:

SuperStrict


For Local i:Int = 0 To 10 
Next

print i ' will not work! (undefined variable)


You can use another variable defined in a broader scope than the loop to save any loop-index values you might be interested in.

It's (imho) a very good practice to use index variables (such as i in the example above) only within the loop, and since BMX supports isolating the variable to the loop this is really preferred.