How is my BlitzMax and OOP programming?
BlitzMax Forums/BlitzMax Programming/How is my BlitzMax and OOP programming?
| ||
| I'm looking to get some tips on my BlitzMax and OOP programming. I'm looking for ways I can improve on and some gotchas I don't know about. I am having a bit of a problem with scaling, so I would appreciate help on that. What the program is suppose to do is load six images into a list. And then cycles thru the list and scale each image 10x its size, along with fading it out and then remove the object from the list. Then when the list is empty start over again.
Strict
Graphics 800,600,0
Global ImageList:TList = New TList
'string array
Global arrPath:String[]=["gfx/blue.bmp","gfx/green.bmp","gfx/yellow.bmp","gfx/red.bmp","gfx/orange.bmp","gfx/violet.bmp"]
'loading path to graphics
'and create objects
Function LoadPieces()
For Local i:String = EachIn arrPath
Image.Create(i)
Next
End Function
Type Image
Global ImageListCount:Int =0
Field xpos,ypos:Int
Field _image:TImage = New TImage
Field AlphaLevel:Int =1.0
'instantiate and each object to List
Function Create:Image(strPath:String)
Local img:Image = New Image
img._image = LoadImage(strPath )
ListAddLast ImageList, img
End Function
'constructor
Method New()
ImageListCount:+1
xpos = (GraphicsWidth() / 2)
ypos = (GraphicsHeight() / 2)
End Method
'go thru each object, scale it and fade object
Method Update()
Global scale:Int
SetBlend ALPHABLEND
For Local i:Image = EachIn ImageList
MidHandleImage i._image
While (1)
Cls
SetScale scale,scale
SetAlpha i.AlphaLevel
DrawImage i._image, (i.xpos), i.ypos
Flip
scale:+1
i.AlphaLevel:-0.5
If scale >= 10 Then
ListRemove ImageList, i
scale=1.0
i.AlphaLevel=1.0
Exit
EndIf
Wend
Next
FlushMem()
'after all objects are destroyed, load objects again and repeat process
Reset()
End Method
Method Reset()
If ListIsEmpty(ImageList) = True Then
LoadPieces()
EndIf
End Method
End Type
Local i:Image = New Image
'main
While Not KeyHit(KEY_ESCAPE)
i.update()
Wend
'destroy i or blitz takes care of this for me when
'the program ends
i=Null
|
| ||
| You've set scale as an Int when it should be a float. Haven't looked at anything else. |
| ||
| Well here you go: You've set scale as an Int when it should be a float. Or double. Also, the same is true for Alpha. Field _image:TImage = New TImage As far as I can tell, this is redundant since all images are stored in a List anyway.Additionally you have a problem with creation responsability. Since New() cannot be made private you should use either New(), or the Create Function (Simple Factory). Moving the lines in the New() method to the Create Function would solve this, and additionally you should only increase the count, and add an object to the list, if image loading was actually successful. Additionally there's a potential problem with the Update method. Does having a Global here actually work? Shouldn't scale be a Field (maybe even a Global/static one) like Alphalevel? Also the update method has way too many different responsabilities. It would be better to seperate the update (queue management, etc.) and actual rendering (fade/scale, drawimage) in seperate methods. Additionally Cls/Flip/Flushmem etc. should be moved to the main loop. The main loop could then look something like: While Not KeyHit(KEY_ESCAPE) Cls i.update() i.render() Flip Flushmem WendOther than that it looks perfectly reasonable. |
| ||
OK, I made some changes. How is this?
Strict
Graphics 800,600,0,20
Type Image
Global ImageList:TList
Field xpos,ypos:Int
Field _image:TImage
Field AlphaLevel:Float
Field Scale:Float
'instantiate and each object to List
Function Create:Image(startXpos:Int,startYpos:Int,imgAlpha:Float,imgScale:Float)
Local arrPath:String[]=["gfx/blue.bmp","gfx/green.bmp","gfx/yellow.bmp","gfx/red.bmp","gfx/orange.bmp","gfx/violet.bmp"]
For Local i:String = EachIn arrPath
Local img:Image = New Image
img._image = LoadImage(i)
img.xpos = startXpos
img.ypos = startYpos
img.AlphaLevel = imgAlpha
img.Scale = imgScale
Next
End Function
Method New ()
If ImageList = Null
ImageList = New TList
EndIf
ImageList.AddLast Self
End Method
'go thru each object, scale it and fade object
Method Update()'img:Image)
Scale:+1.0
AlphaLevel:-0.1
End Method
Function Render()
SetBlend ALPHABLEND
If ((ImageList = Null) Or (ImageList.Count() = 0)) Then Return
Local i:Image = Image(ImageList.First())
MidHandleImage i._image
SetScale i.Scale,i.Scale
SetAlpha i.AlphaLevel
DrawImage i._image, (i.xpos), i.ypos
If i.Scale >= 15.0 Then
ListRemove ImageList,i
i.Scale=1.0
i.AlphaLevel=1.0
EndIf
i.Update()
End Function
End Type
'main
While Not KeyHit(KEY_ESCAPE)
Cls
If MouseHit(1) Then
Image.Create((GraphicsWidth() / 2),(GraphicsHeight() / 2),1.0,1.0)
EndIf
Image.Render()
Flip()
FlushMem()
Wend
|
| ||
| Method Update(img:Image) shouldn't have an img parameter. change ImageList to "List" and put it inside Image remove "ImageListCount" Either make Method Render() into a function or take out the loop Looks like youve missed the point of oop, noofense. Prolly see more changes once these are put in. |
| ||
| No offense taken. That's why I asked. Now tell me how am I missing the point of OOP. I'm trying to make sure I didn't have the attitude that I understood OOP when I really didn't. |
| ||
| Bump. |
| ||
| Actually, it looks fine now, its just that it appeared that you were passing img since in a procedural program this is what you would do. Instead, when making methods, you dont pass the object that is being operated on. Also, if you have a function where you pass in a type to modify, particularly if it is a function within a type, it should probably be a method instead. |
| ||
| OK, Thanks! |
   |