Freedom Fighter - Rise of humans WORK IN PROGRESS

Ascended (10819)

25-04-2019, 23:29

What’s the problem exactly right now? I’m not sure what to be looking for…

But regardless, I think the core of the problem is that the code isn’t written in a way to be understood easily. This makes it hard for both you and me to understand what is wrong.

The code would be easier to read if you used ix to point to the player, and iy to the current enemy sprite, and used those indexed registers instead of hl. Because you do not need to juggle with the address pointer (ld l,0, push/pop hl, inc l, etc.), and from the index it can immediately be seen what does what, it becomes shorter and easier to understand, and easier to debug.

When you do use the index registers, don’t use (ix + 0), (ix + 1), but use symbols for the offsets, like (ix + sprite_y), (ix + sprite_x), etc. This way you remove the need for a comment about the meaning, because the symbol now describes what’s happening. E.g. in “ld a,(iy + sprite_y) ; load enemy ypos in a”, there comment has become superfluous.

Additionally, I would change all the conditional jumps so that you can put all checks underneath each other, jumping to “next” (moved to the bottom) when they don’t match, so that the execution flow is easier to understand.

Lastly, I would extract the bounds checks to a subroutine which returns in carry whether there is a collision between the sprites pointed to by ix and iy. So that the collision checking becomes separated from the looping and exploding.

By Grauw

Ascended (10819)

26-04-2019, 00:34

Something like:

sprite_y: equ 0
sprite_x: equ 1
sprite_pat: equ 2
sprite_col_ec: equ 3

you_are_dead: equ 0e5cfh



ship_collision:
	ex	af,af'
	exx
	ld	a,0
	ld	(you_are_dead),a
	ld	ix,ramspttbl	; player sprite attributes
	ld	iy,ramspttbl+48	; first enemy (sprite 12) attributes
	ld	hl,actpat+36	;point to active flag of sprite 12 (first enemy sprite)
	ld	b,20	;max enemy sprites (from 12 to 31)
loop:
	bit	0,(hl)
	jp	z,next
	call	check_collision
	call	nc,kill_enemy
next:
	inc	hl	;increment hl to point to next sprites infos
	inc	hl	;in actpat table (3 values for sprite)
	inc	hl	;1st: 1/0 active/unactive 2nd: pattern movement nr 3:pattern position
	ld	a,iyl	;increment iy to point to next sprite in SAT (ramspttbl)
	add	a,4
	ld	iyl,a
	djnz	loop	;repeat until checked all enemies sprites
	ld	a,(you_are_dead)
	or	a
	call	nz,kill_player
	exx
	ex	af,af'
	ret



kill_enemy:
	ld	(iy + sprite_pat),72	;set first frame of axplosion sprite
	ld	a,(iy + sprite_col_ec)
	or	11	;change only color for explosion (leave EC as is)
	ld	(iy + sprite_col_ec),a
	inc	hl	;point to second actpat value (pattern nr)
	ld	(hl),7	;set it as 7 (explosion animation)
	inc	hl	;point to next actpat value (pattern frame)
	ld	(hl),0	;set it to 0 (first frame)
	dec	hl
	dec	hl
	ld	a,1
	ld	(you_are_dead),a	;set you_are_dead flag
	ret

kill_player:
	ld	(ix + sprite_y),190
	ld	(ix + sprite_x),119
	ld	(ix + 4 + sprite_y),190	; same for player ship second color sprite
	ld	(ix + 4 + sprite_x),119
	ret



; ix = player sprite attributes
; iy = enemy sprite attributes
; f <- c: no collision, nc: collision
check_collision:
check_upper_y:
	ld	a,(iy + sprite_y)
	add	a,-14	;add -14 for upper line of collision rectangle
	ld	c,a	;upper line in c
	ld	a,(ix + sprite_y)
	cp	c	;cp player ypos with upperline of rectangle
	ret	c
check_bottom_y:
	ld	c,a	;player ypos in c
	ld	a,(iy + sprite_y)
	add	a,14	;add 14 for bottom line of collision rectangle
	cp	c
	ret	c
check_left_x:
	ld	a,(iy + sprite_x)
	add	a,-14	;set most left axis for collision zone
	bit	7,(iy + sprite_col_ec)	; early clock set?
	jp	z,no_left_ec
	sub	32	;if so subtract 32 from enemy xpos
no_left_ec:
	ld	c,a	;enemy xpos in c
	ld	a,(ix + sprite_x)
	cp	c	;compare player xpos with left x of collision rectangle
	ret	c
check_right_x:
	ld	c,a	;player xpos in c
	ld	a,(iy + sprite_x)
	add	a,14	;set right border of collision rectangle
	bit	7,(iy + sprite_col_ec)	; early clock set?
	jp	z,no_right_ec
	sub	32
no_right_ec:
	cp	c
	ret

Note I was quick and not very careful with these changes nor did I test, so I may have introduced issues rather than fixed any, but this should illustrate what I mean by improving the structure of the code.

By thegeps

Paragon (1249)

25-04-2019, 23:56

Airship collision (the routine I pasted here) works right
Problems are:
-I have 2 sprites for every enemy: sometimes only one of them explode and other sprite (sometimes inner color, sometimes black border) doesn't disappear. It's strange 'cause the routine check for all enemy sprites so them both have to explode (and it happen most of times)
- when checking collision with diagonal shots (both left and right) something doesn't work correctly (some enemies explode without shots fired them)
I use the same algorithm for all collision routines (same code with minor differences: in ship collision set a flag for player death and respawn, in shots collisions routine the flag imdicate if an enemy has be fired, so at the end of the check cycle for ALL enemies spritea, based on flag player bullet disappear or not). And at the start of the shots routines is checked if current checked sprite is an explosion due to skip the check)

By thegeps

Paragon (1249)

26-04-2019, 00:06

I see your code is more readable. I try to not use ix and iy for speed reasons. And actpat and ramspttbl are 256 aligned, so I can use simply inc e, inc l to move forward or ld e,n ld l,n to move where I want (step of 4 in ramspttbl that is the SAT in RAM and step of 3 for actpat activeflag/movement pattern nr/in pattern position)

By thegeps

Paragon (1249)

26-04-2019, 00:09

In the ship_collision routine (the one I pasted here) player death and enemy death are the same. In fact it checks collision between player ship and enemies and if ot occours kill both enemy and player airship. So kill_player and kill_enemy can be merged

By Grauw

Ascended (10819)

26-04-2019, 00:49

thegeps wrote:

I see your code is more readable. I try to not use ix and iy for speed reasons.

I think there’s a lot of bad advice out there regarding the use of index registers. When I was a beginner I also avoided them due to that bad advice. Now I embrace them, and my programming life is much better .

Firsty it’s premature optimisation; when the code is not working yet there is no point already optimising it. Optimisation is obfuscation, it makes implementation slower, buggier, code harder to read, and micro-optimisations like this make it more difficult to recognise algorithmic optimisations which usually give a lot more speed gain.

Secondly for the type of access you do here, index registers will be comparable in speed or may even be faster. By adding indexed access you can remove a lot of other instructions. Additionally it frees up hl which can be used then instead of de, saving even more instructions (in kill_enemy amongst others, also the ld a,(de) / or a could be changed to bit 0,(hl)).

Thirdly I see many other opportunities for optimisation that do not harm the code readability, e.g. jr nc,check_left_x / jp to_next / check_left_x: could be optimised to jr c,to_next. The collision check itself can also be optimised by comparing the co-ordinates just once and then checking the difference. Focusing on avoiding ix and iy (or subroutine calls) just makes it difficult to see the flow of the code and find the real (algorithmic) optimisations.

And lastly, these optimisations make it almost impossible for me to assist you figure out this bug, because as someone not involved in the code it becomes *really* hard to understand what’s going on when table address pointers are constantly changing and the code flow jumps all over the place (“goto considered harmful” is a well known expression).

By Grauw

Ascended (10819)

26-04-2019, 00:52

thegeps wrote:

They can be. I figured the you_are_dead flag was to prevent killing the player more than once when there are collisions with multiple enemies. However that probably doesn’t happen frequently enough that it actually matters much, so the code could be simplified by combining them and removing the flag (as long as it’s ok to kill the player multiple times).

By thegeps

Paragon (1249)

26-04-2019, 00:51

Thx for your advices. I'll try your code and to use imdexes registers.drom now on!

By thegeps

Paragon (1249)

26-04-2019, 00:53

Simply I don't get why if my code works for airship vs enemies collision and central bullet vs enemies collision it doesn't work for diagonal bullets vs enemies... It's the SAME code... O_o

By Grauw

Ascended (10819)

26-04-2019, 01:03

Are the diagonal bullet table offsets correct in all the places? You may have forgotten to change it after copy/pasting.

Since you use the 8-bit LSB increment optimisation, are the tables for sure aligned correctly?

Is the collision never working or is it having problems perhaps at the edges of the screen, possibly indicating issues with X overflow (icm. early clock)?

Freedom Fighter - Rise of humans *WORK IN PROGRESS*

Freedom Fighter - Rise of humans WORK IN PROGRESS