Why doesn't this code work?

Monkey Forums/Monkey Beginners/Why doesn't this code work?

Impmaster

(Posted 2014) [#1]

Field X:Int = 2

Method Test()
x = x - 1
End

When I run this, I get "memory access violation"

Xaron

(Posted 2014) [#2]

I doubt that you can even compile this. ;)

First. Everything is case sensitive. So having a field "X" and decreasing "x" won't work.

"Working" example:

Function Main()
  Local a:A = New A()
  a.Test()
End Function

Class A
  Field x:Int = 2

  Method Test()
    x = x - 1
  End Method
End Class

Impmaster

(Posted 2014) [#3]

Okay, what I wrote was just a simplified version of my real code. In my real code, the versions are the same, and it compiles but does not run.

Xaron

(Posted 2014) [#4]

So please post your real code then.

Impmaster

(Posted 2014) [#5]


Class Bullet

Field reloadTimer:Int = 10

Method checkAll()
    	
    	reloadTimer = reloadTimer - 1

    
End

End

That's the part that's giving me trouble. Maybe it's something else and I just made a complicated mistake, but until I put this in, it all worked. If this isn't enough info, I'll post the whole file tomorrow.

Gerry Quinn

(Posted 2014) [#6]

It looks fine.

Are you calling checkAll() using a valid instance of Bullet?

If bullet is Null, then bullet.checkAll() will give a run-time error - the actual error might depend on the target and whether you are in release or debug mode.

Impmaster

(Posted 2014) [#7]

Import mojo


Class Bullet
    
    Field x:Float
    Field y:Float
    Field direction:Float
    Field speed:Float = 10
    
    Field image:Image
    
    Field reloadTimer:Int = 10
    Field startTimer:Int = 10
    Field canShoot:Bool = True
    
    
    Global bulletList:List<Bullet> = New List<Bullet>
    
    Method New (startX:Float, startY:Float, startDirection:Float)
        x = startX
        y = startY
        direction = startDirection
        
        image = LoadImage("bullet.png")

    End
    
    
    Method Fire (startX:Float, startY:Float, startDirection:Float)
    	If canShoot = True Then bulletList.AddLast New Bullet(startX, startY, startDirection)
    End
    
    Method checkAll()
    	
    	'reloadTimer = reloadTimer - 1
    	'If reloadTimer = 0 Then canShoot = True; reloadTimer = startTimer
	    	
    	For Local bullet:Bullet = Eachin bulletList
    		bullet.checkOne()
    	Next

    
    End
    
    Method drawAll()
    	
    	For Local bullet:Bullet = Eachin bulletList
    		bullet.drawOne()
    	Next
    
    End
    
    Method drawOne ()
        DrawImage image, x, y
    End
    
    Method checkOne ()
    	x -= Sin(direction) * speed 
		y -= Cos(direction) * speed
    End
    


	


End

This is the whole file that I have. I have another class that runs bullet.checkAll() every update loop. What's wrong?

Impmaster

(Posted 2014) [#8]

Wait, I managed to fix my problem by making all the Fields (reloadTimer, startTimer, and canShoot) into Globals.

Can someone explain why that works, and how to make it a little cleaner? I won't need these anywhere else in the program, so I'd prefer them not being Global.

AdamRedwoods

(Posted 2014) [#9]

CheckAll() should be a function, not a method.

with a method you are only updating one instance of the timer, rather than a singular global instance of the timer.

Impmaster

(Posted 2014) [#10]

If I make it a function, and I make reloadTimer a Field, then it says that I cannot access reloadTimer from here.

computercoder

(Posted 2014) [#11]

I think you've crossed a few things in the organization of the class.

The bullet list, in my opinion, should not be a part of the class that it is holding. Instantiate that in a class outside of bullet.

The three fields reloadTimer, startTimer, and canShoot also do not belong in the class. They don't make sense in this class, they too should be in another class with the bullet list object.

I would break this into two separate classes: Bullet and for lack of a better name, BulletManager (name it whatever, but this is just to demonstrate what I am saying here).

The bullet class would look something like this:

Class Bullet

    Field x:Float = 0
    Field y:Float = 0
    Field direction:Float = 0
    Field speed:Float = 10
    
    Field image:Image = null

    Method New (startX:Float, startY:Float, startDirection:Float)
			
			x = startX
			y = startY
			direction = startDirection
			
			image = LoadImage("bullet.png")

    End
    
		Method Draw()
			DrawImage image, x, y
		End
		
		Method Update()
 
			x -= Sin(direction) * speed 
			y -= Cos(direction) * speed
		
			' You should also add some way to detect a collision
			' Or see if the bullet moved off screen, etc
			' That way the list can have the bullet removed
		End
		
End

And the BulletManager class something like this:

Class BulletManager

	Private
	Field reloadTimer:Int = 10
	Field startTimer:Int = 10
	Field canShoot:Bool = True
 
    
	Public
	Field bulletList:List<Bullet> = New List<Bullet>
	
	Method Fire(startX:Float, startY:Float, startDirection:Float)

		If canShoot = True 
			bulletList.AddLast (New Bullet(startX, startY, startDirection))
		End

	End
    	
	Method Draw()
	
		For Local bullet:Bullet = Eachin bulletList
			bullet.Draw()
		Next

	End

	Method Update()
	
		reloadTimer -= 1
		
		If reloadTimer = 0 
			canShoot = True
			reloadTimer = startTimer
		End
		
		For Local bullet:Bullet = Eachin bulletList
			bullet.checkOne()
		Next

	End
	
End

I didn't change out a lot of the code, just basically divided them into logical classes, The idea is that the BulletManager creates a bullet and adds it to the list to store it. The Update method of the class will call each bullet in the list and execute its Update method to move it, animate, etc. TThe same idea applies to the Draw() method.

So in a nutshell, you create an object in your game that has the bullet manager class. from there, you use that object to deal with creating bullets and moving them around. There are plenty of opportunities to improve the structure of this code. I did not want to change it too much so you could easily follow what I was trying to show you here.

Hopefully this helps you.

As for the Global vs Field issue, I'm not entirely sure, but it could have to do with the scope of the list object, and what Monkey-X was seeing it as vs what you thought it was. To me, a Global is like a static variable. Its created once in the class (not per object) and called as such. the lesser Public level fields are part of the instantiated object for the class. This behaves differently from the global level object in that the fields can be only accessed by the instantiated object and not through the class itself.

Gerry Quinn

(Posted 2014) [#12]

Having bulletList 'belong' to Bullet is asking for trouble. Although I can't be certain, I suspect you are somehow calling checkAll() when bulletList is empty.

Making them global ensures that they exist when there are no instances of Bullet, but is still not a great solution IMO - I could see it for bulletList and bulletImage, but the reload timers etc. are too much for good style. Still, the computer doesn't care and it depends on your coding style and the scope of your project.

bulletImage, the timers, canShoot, bulletList and checkAll() all belong outside of Bullet. In a BulletManager class, or just in a game control class, it doesn't matter. You can make them global if you must, but move them right out of the class scope if you are doing so, or you will tie yourself in knots.

Basically, what computercoder says.

Impmaster

(Posted 2014) [#13]

Ah okay. I was given that solution in http://www.monkey-x.com/Community/posts.php?topic=8296

Midimaster

(Posted 2014) [#14]

The bulletList can stay inside the Class as long as it is GLOBAL.

The difference between GLOBALs and FIELDs inside Classes is, that FIELDs descripe independent properties of each single bullet, while GLOBALs descripe common properties: All bullet have the same value.

Example: Class DOG.... "Having 4 legs" should be a GLOBAL, but the dogs size should be a FIELD.

The difference between FUNCTION and METHOD... A Function is called when care about the sum of elements: Adding new one, deleting one, Drawing all, etc... A Method is necessary if a single existing element should be changed, drawn, checked etc...

Inside a FUNCTION you can not access directly to individual FIELD values. Inside METHODS you can acces the GLOBALs too.

Your "reloadTimer" is defined as FIELD and not accessible from FUNCTION.
"reloadTimer" is not a property of each individual bullet, but of the gun!!! So you have to change "reloadTimer" to GLOBAL.

Class Bullet
     Field x:Float, y:Float, direction:Float
    Global speed:Float = 10, image:Image
    Global reloadTimer:Int = 10

I think also "speed" and "image" should be GLOBAL, because all bullets use the same image and have the same speed....

dawlane

(Posted 2014) [#15]

The bulletList can stay inside the Class as long as it is GLOBAL.

And as long as you do not intend to extend the class. Any operations on a global will affect the Global in it's parent class, unless the child class overrides it's parents Global. You could say that a Global in a Class is the equivalent to declaring a variable as static in C/C++.

computercoder

(Posted 2014) [#16]

All I am going to say about using GLOBALS in classes is to be sure of what you are doing and take care with them just so that you do not accidentally introduce a bug. It is, in my opinion and many of my co-workers opinions,that you should not attempt making classes that feel confusing or look strange like that. Programming gives you all the flexibility you want and you can go about it however you like, but that style is very strange. I mean when OOP was thought about, I really doubt that they thought it would be a great idea to build a brick from another brick and store it within the same brick it was created from. Logically an incorrect way of structuring it, even if it were completely possible and worked (in the end) to have the overall same effect. I look at how the code will be to read for me 6 months later when I need to revisit it, or when I have another programmer jump on board to get the job done. The less confusion there is in the organization, the better it is getting changes made later, or debugging becomes.

Samah

(Posted 2014) [#17]

@Gerry Quinn: Having bulletList 'belong' to Bullet is asking for trouble.

This.

Personally I hate the idea of keeping a class's instances in a global list in the same class. What if I want more than one list? Then I have to ignore the global one, or write code somewhere else that will do the same loops that any functions might. It just becomes really messy.