Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name the unnamed hram variables and one wram label #438

Merged
merged 11 commits into from
Nov 20, 2023

Conversation

Vortyne
Copy link
Contributor

@Vortyne Vortyne commented Nov 20, 2023

hFFDC goes completely unused so does it even need to be declared in the union?

@Vortyne Vortyne changed the title Name the unnamed hram variables Name the unnamed hram variables and one wram label Nov 20, 2023
ram/hram.asm Outdated Show resolved Hide resolved
ram/wram.asm Outdated Show resolved Hide resolved
ram/hram.asm Outdated Show resolved Hide resolved
@@ -80,7 +80,7 @@ DetectCollisionBetweenSprites:
and $f0
or c

ldh [hFF90], a ; store Y coordinate adjusted for direction of movement
ldh [hSpriteCollisionsTempYStore], a ; store Y coordinate adjusted for direction of movement
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete comments when labels make them obsolete.

Suggested change
ldh [hSpriteCollisionsTempYStore], a ; store Y coordinate adjusted for direction of movement
ldh [hSpriteCollisionsTempYStore], a

Copy link
Contributor Author

@Vortyne Vortyne Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these y coord and x coord stores are used for 2 purposes

  • storing y or x coord adjusted for direction of movement
  • storing the distance between the two sprites' adjusted y or x values

so I figured these comments would help distinguish, since wSpriteCollisionsTempYCoordOrDistanceBetweenSpriteAdjustedYValue
would be a bit much...hence why I named it hSpriteCollisionsTempYStore initially, due to multiple purposes, all being dealing with Y or X values

Copy link
Contributor Author

@Vortyne Vortyne Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I should make them into two?
hSpriteCollisionsTempYCoord::
hSpriteCollisionsDistanceBetweenAdjustedYValues:: db

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I posted some more label suggestions after reading the file more carefully. Thanks for your patience with all the feedback.

ram/hram.asm Outdated
@@ -330,8 +330,7 @@ hGymTrashCanRandNumMask::
db

NEXTU
hFFDB:: db
hFFDC:: db
hInteractedWithBookshelf:: db ; set to 0 if you interacted with a bookshelf, $FF if not when attempting to interact with a hidden object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
hInteractedWithBookshelf:: db ; set to 0 if you interacted with a bookshelf, $FF if not when attempting to interact with a hidden object
; when attempting to interact with a hidden object,
; set to 0 if you interacted with a bookshelf, $FF if not
hInteractedWithBookshelf:: db

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also since hFFDC is gone now, this can merge with the union above it (and I think we don't need the comment after all):

NEXTU
hItemCounter::
hSavedCoordIndex::
hMissableObjectIndex::
hInteractedWithBookshelf::
hGymTrashCanRandNumMask::
	db
ENDU

@@ -296,7 +296,7 @@ DetectCollisionBetweenSprites:
; to indicate which sprite the collision occurred with
inc l
inc l
ldh a, [hFF8F] ; a = loop counter
ldh a, [hSpriteCollisionsLoopCounter]
ld de, SpriteCollisionBitTable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data for SpriteCollisionBitTable should use a loop:

SpriteCollisionBitTable:
FOR n, 16
	bigdw 1 << n
ENDR

(The bigdw macro will need porting from pokecrystal.)

ram/hram.asm Outdated
hFF90:: db
hFF91:: db
hFF92:: db
hSpriteCollisionsLoopCounter:: db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used as a 0-15 offset into SpriteCollisionBitTable for the sprite being collision-tested with the "current sprite" (whose corresponding offset is already named hCurrentSpriteOffset).

Suggested change
hSpriteCollisionsLoopCounter:: db
hCollidingSpriteOffset:: db

ram/hram.asm Outdated
hFF91:: db
hFF92:: db
hSpriteCollisionsLoopCounter:: db
hSpriteCollisionsTempYCoord:: db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First this is initialized to [i#SPRITESTATEDATA1_YPIXELS] with some +/-7 adjustment, and stored in [i#SPRITESTATEDATA1_YADJUSTED]. Then the absolute difference between [i#SPRITESTATEDATA1_YADJUSTED] and [j#SPRITESTATEDATA1_YADJUSTED] is stored in it , and used to calculate hSpriteCollisionsAdjustedDistance. It also has a "7 or 9 depending on sprite i's delta Y". I think it has enough overlapping purposes in the collision code that it can be thought of as a generic "Y-related temp value".

Suggested change
hSpriteCollisionsTempYCoord:: db
hCollidingSpriteTempYValue:: db

ram/hram.asm Outdated
hFF92:: db
hSpriteCollisionsLoopCounter:: db
hSpriteCollisionsTempYCoord:: db
hSpriteCollisionsTempXCoord:: db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
hSpriteCollisionsTempXCoord:: db
hCollidingSpriteTempXValue:: db

ram/hram.asm Outdated
hSpriteCollisionsLoopCounter:: db
hSpriteCollisionsTempYCoord:: db
hSpriteCollisionsTempXCoord:: db
hSpriteCollisionsAdjustedDistance:: db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used for both the Y and X distance, so:

Suggested change
hSpriteCollisionsAdjustedDistance:: db
hCollidingSpriteAdjustedDistance:: db

Copy link
Member

@Rangi42 Rangi42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! Getting closer to 100% :D

@Rangi42 Rangi42 merged commit 7a7a6d6 into pret:master Nov 20, 2023
1 check passed
@Vortyne Vortyne deleted the patch-3 branch August 30, 2024 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants