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.

Sign up for GitHub

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

Rename unused items #424

Closed
wants to merge 1 commit into from
+31 −50
Closed

Rename unused items #424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions constants/item_constants.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
const POKE_BALL ; $04
const TOWN_MAP ; $05
const BICYCLE ; $06
const SURFBOARD ; $07
const SURF_ITEM ; $07
SatoMew marked this conversation as resolved.
Show resolved Hide resolved
const SAFARI_BALL ; $08
const POKEDEX ; $09
const MOON_STONE ; $0A
Expand Down Expand Up @@ -52,13 +52,13 @@ DEF SAFARI_ROCK EQU $16 ; overload
const DOME_FOSSIL ; $29
const HELIX_FOSSIL ; $2A
const SECRET_KEY ; $2B
const UNUSED_ITEM ; $2C "?????"
const DUMMY_ITEM ; $2C ; unused
SatoMew marked this conversation as resolved.
Show resolved Hide resolved
SatoMew marked this conversation as resolved.
Show resolved Hide resolved
const BIKE_VOUCHER ; $2D
const X_ACCURACY ; $2E
const LEAF_STONE ; $2F
const CARD_KEY ; $30
const NUGGET ; $31
const PP_UP_2 ; $32
const PP_UP_2 ; $32 ; unused, dummy
SatoMew marked this conversation as resolved.
Show resolved Hide resolved
const POKE_DOLL ; $33
const FULL_HEAL ; $34
const REVIVE ; $35
Expand Down
4 changes: 2 additions & 2 deletions data/items/key_items.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ KeyItemFlags:
dbit FALSE ; POKE_BALL
dbit TRUE ; TOWN_MAP
dbit TRUE ; BICYCLE
dbit TRUE ; SURFBOARD
dbit TRUE ; SURF_ITEM
dbit TRUE ; SAFARI_BALL
dbit TRUE ; POKEDEX
dbit FALSE ; MOON_STONE
Expand Down Expand Up @@ -43,7 +43,7 @@ KeyItemFlags:
dbit TRUE ; DOME_FOSSIL
dbit TRUE ; HELIX_FOSSIL
dbit TRUE ; SECRET_KEY
dbit TRUE ; UNUSED_ITEM
dbit TRUE ; DUMMY_ITEM
dbit TRUE ; BIKE_VOUCHER
dbit FALSE ; X_ACCURACY
dbit FALSE ; LEAF_STONE
Expand Down
6 changes: 3 additions & 3 deletions data/items/names.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ItemNames::
li "POKé BALL"
li "TOWN MAP"
li "BICYCLE"
li "?????"
li "?????" ; SURF_ITEM
li "SAFARI BALL"
li "POKéDEX"
li "MOON STONE"
Expand Down Expand Up @@ -43,13 +43,13 @@ ItemNames::
li "DOME FOSSIL"
li "HELIX FOSSIL"
li "SECRET KEY"
li "?????"
li "?????" ; DUMMY_ITEM
li "BIKE VOUCHER"
li "X ACCURACY"
li "LEAF STONE"
li "CARD KEY"
li "NUGGET"
li "PP UP"
li "PP UP" ; PP_UP_2
li "POKé DOLL"
li "FULL HEAL"
li "REVIVE"
Expand Down
6 changes: 3 additions & 3 deletions data/items/prices.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ItemPrices::
bcd3 200 ; POKE_BALL
bcd3 0 ; TOWN_MAP
bcd3 0 ; BICYCLE
bcd3 0 ; SURFBOARD
bcd3 0 ; SURF_ITEM
bcd3 1000 ; SAFARI_BALL
bcd3 0 ; POKEDEX
bcd3 0 ; MOON_STONE
Expand Down Expand Up @@ -43,13 +43,13 @@ ItemPrices::
bcd3 0 ; DOME_FOSSIL
bcd3 0 ; HELIX_FOSSIL
bcd3 0 ; SECRET_KEY
bcd3 0 ; XXX
bcd3 0 ; DUMMY_ITEM
bcd3 0 ; BIKE_VOUCHER
bcd3 950 ; X_ACCURACY
bcd3 2100 ; LEAF_STONE
bcd3 0 ; CARD_KEY
bcd3 10000 ; NUGGET
bcd3 9800 ; XXX PP_UP
bcd3 9800 ; PP_UP_2
bcd3 1000 ; POKE_DOLL
bcd3 600 ; FULL_HEAL
bcd3 1500 ; REVIVE
Expand Down
4 changes: 2 additions & 2 deletions data/moves/animations.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ AttackAnimationPointers:
dw JumpKickAnim
dw RollingKickAnim
dw SandAttackAnim
dw HeadButtAnim
dw HeadbuttAnim
dw HornAttackAnim
dw FuryAttackAnim
dw HornDrillAnim
Expand Down Expand Up @@ -363,7 +363,7 @@ SandAttackAnim:
battle_anim SAND_ATTACK, SUBANIM_1_SAND, 1, 6
db -1 ; end

HeadButtAnim:
HeadbuttAnim:
battle_anim HEADBUTT, SUBANIM_1_STAR_BIG, 1, 6
db -1 ; end

Expand Down
28 changes: 16 additions & 12 deletions engine/items/item_effects.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ ItemUsePtrTable:
dw ItemUseBall ; POKE_BALL
dw ItemUseTownMap ; TOWN_MAP
dw ItemUseBicycle ; BICYCLE
dw ItemUseSurfboard ; out-of-battle Surf effect
dw ItemUseSurf ; SURF_ITEM
dw ItemUseBall ; SAFARI_BALL
dw ItemUsePokedex ; POKEDEX
dw ItemUseEvoStone ; MOON_STONE
Expand Down Expand Up @@ -60,19 +60,19 @@ ItemUsePtrTable:
dw UnusableItem ; DOME_FOSSIL
dw UnusableItem ; HELIX_FOSSIL
dw UnusableItem ; SECRET_KEY
dw UnusableItem
dw UnusableItem ; DUMMY_ITEM
dw UnusableItem ; BIKE_VOUCHER
dw ItemUseXAccuracy ; X_ACCURACY
dw ItemUseEvoStone ; LEAF_STONE
dw ItemUseCardKey ; CARD_KEY
dw UnusableItem ; NUGGET
dw UnusableItem ; ??? PP_UP
dw ItemUsePokedoll ; POKE_DOLL
dw UnusableItem ; PP_UP_2
dw ItemUsePokeDoll ; POKE_DOLL
dw ItemUseMedicine ; FULL_HEAL
dw ItemUseMedicine ; REVIVE
dw ItemUseMedicine ; MAX_REVIVE
dw ItemUseGuardSpec ; GUARD_SPEC
dw ItemUseSuperRepel ; SUPER_REPL
dw ItemUseSuperRepel ; SUPER_REPEL
dw ItemUseMaxRepel ; MAX_REPEL
dw ItemUseDireHit ; DIRE_HIT
dw UnusableItem ; COIN
Expand All @@ -89,13 +89,13 @@ ItemUsePtrTable:
dw ItemUseOaksParcel ; OAKS_PARCEL
dw ItemUseItemfinder ; ITEMFINDER
dw UnusableItem ; SILPH_SCOPE
dw ItemUsePokeflute ; POKE_FLUTE
dw ItemUsePokeFlute ; POKE_FLUTE
dw UnusableItem ; LIFT_KEY
dw UnusableItem ; EXP_ALL
dw ItemUseOldRod ; OLD_ROD
dw ItemUseGoodRod ; GOOD_ROD
dw ItemUseSuperRod ; SUPER_ROD
dw ItemUsePPUp ; PP_UP (real one)
dw ItemUsePPUp ; PP_UP
dw ItemUsePPRestore ; ETHER
dw ItemUsePPRestore ; MAX_ETHER
dw ItemUsePPRestore ; ELIXER
Expand Down Expand Up @@ -665,8 +665,8 @@ ItemUseBicycle:
.printText
jp PrintText

; used for Surf out-of-battle effect
ItemUseSurfboard:
; indirectly used by SURF in StartMenu_Pokemon.surf
ItemUseSurf:
ld a, [wWalkBikeSurfState]
ld [wWalkBikeSurfStateCopy], a
cp 2 ; is the player already surfing?
Expand Down Expand Up @@ -1427,6 +1427,8 @@ VitaminNoEffectText:

INCLUDE "data/battle/stat_names.asm"

; for BOULDERBADGE when used from ITEM window aka SAFARI_BAIT
; BOULDERBADGE behaves differently in CeruleanBadgeHouseMiddleAgedManText
SatoMew marked this conversation as resolved.
Show resolved Hide resolved
ItemUseBait:
ld hl, ThrewBaitText
call PrintText
Expand All @@ -1437,6 +1439,8 @@ ItemUseBait:
ld de, wSafariEscapeFactor ; escape factor
jr BaitRockCommon

; for CASCADEBADGE when used from ITEM window aka SAFARI_ROCK
; CASCADEBADGE behaves differently in CeruleanBadgeHouseMiddleAgedManText
ItemUseRock:
ld hl, ThrewRockText
call PrintText
Expand Down Expand Up @@ -1482,7 +1486,7 @@ ThrewRockText:
text_far _ThrewRockText
text_end

; also used for Dig out-of-battle effect
; indirectly used by DIG in StartMenu_Pokemon.dig
ItemUseEscapeRope:
ld a, [wIsInBattle]
and a
Expand Down Expand Up @@ -1597,7 +1601,7 @@ ItemUseCardKey:

INCLUDE "data/events/card_key_coords.asm"

ItemUsePokedoll:
ItemUsePokeDoll:
ld a, [wIsInBattle]
dec a
jp nz, ItemUseNotTime
Expand Down Expand Up @@ -1662,7 +1666,7 @@ ItemUseXStat:
ld [hl], a ; restore [wPlayerMoveNum]
ret

ItemUsePokeflute:
ItemUsePokeFlute:
ld a, [wIsInBattle]
and a
jr nz, .inBattle
Expand Down
2 changes: 1 addition & 1 deletion engine/menus/start_sub_menus.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ StartMenu_Pokemon::
bit 1, [hl]
res 1, [hl]
jp z, .loop
ld a, SURFBOARD
ld a, SURF_ITEM
ld [wcf91], a
ld [wPseudoItemID], a
call UseItem
Expand Down
25 changes: 1 addition & 24 deletions home/item.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,7 @@
; uses an item
; UseItem is used with dummy items to perform certain other functions as well
; INPUT:
; [wcf91] = item ID
; OUTPUT:
; [wActionResultOrTookBattleTurn] = success
; 00: unsuccessful
; 01: successful
; 02: not able to be used right now, no extra menu displayed (only certain items use this)
; also used alongside [wPseudoItemID]
UseItem::
farjp UseItem_

; confirms the item toss and then tosses the item
; INPUT:
; hl = address of inventory (either wNumBagItems or wNumBoxItems)
; [wcf91] = item ID
; [wWhichPokemon] = index of item within inventory
; [wItemQuantity] = quantity to toss
; OUTPUT:
; clears carry flag if the item is tossed, sets carry flag if not
Copy link
Member

Choose a reason for hiding this comment

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

Why delete these descriptions? Explaining input and output is nice actually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These comments are misplaced since they refer to UseItem_, TossItem_, and IsKeyItem_, respectively. See also #302.

Copy link
Member

Choose a reason for hiding this comment

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

Comments which describe the input/output of a routine are not at all the type of redundant comments referred to by #302.

That being said, these description comments are also already repeated on TossItem_, IsKeyItem_, so removing them from here could be fine, but the description on UseItem is not repeated on UseItem_.

That being said, it's kind of nice for the description to be on the entrypoint in home/ rather than the implementation in engine/, since the entrypoints are kind of like the "api" of home; convenient to access and convenient to read.

I really like the way this approach is used in poketcg for example:
https://github.com/pret/poketcg/blob/43b7b92/src/home/coin_toss.asm#L1-L13

The comments are not "misplaced" as you say; they are absolutely true in reference to the routines where they are placed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comments which describe the input/output of a routine are not at all the type of redundant comments referred to by #302.

Do current pokegold and pokecrystal feature similar comments? If not, should they? It'd be nice to have a standardized approach across the different projects...

That being said, these description comments are also already repeated on TossItem_, IsKeyItem_, so removing them from here could be fine, but the description on UseItem is not repeated on UseItem_.

That being said, it's kind of nice for the description to be on the entrypoint in home/ rather than the implementation in engine/, since the entrypoints are kind of like the "api" of home; convenient to access and convenient to read.

Would you like to add the comment in UseItem_ or to restore the deleted comments and remove the duplicates from the implementation routines?

Copy link
Member

Choose a reason for hiding this comment

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

I could be persuaded both ways. I see pros and cons to both.

Taking the coin toss example from poketcg, the implementation routine has no description comment on top, which makes sense to me since the code more so speaks for itself:
https://github.com/pret/poketcg/blob/43b7b92/src/engine/duel/core.asm#L7817

Although maybe it would be better if the implementation routine at least had a tl;dr description or something, I dunno.

I'm also not super concerned with standardizing, given that many different people work on the different disassemblies, and the games themselves are structured differently enough that it may be best to consider many things on an individual level. Of course, consistency and uniformity when it's easy is good.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the code is simple or speaks for itself; then it doens't really need a comment imo.

I prefer routine input/output/description comments to go near the routine doing the majority of the "work". Especially if home is only being used as a way to jump far. If the home jump moves registers around, then it could have an additional small comment stating so, but most of the time this should be painfully obvious and simple to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like there is conflicting feedback about this. What should I do?

TossItem::
ldh a, [hLoadedROMBank]
push af
Expand All @@ -31,13 +15,6 @@ TossItem::
ld [MBC1RomBank], a
ret

; checks if an item is a key item
; INPUT:
; [wcf91] = item ID
; OUTPUT:
; [wIsKeyItem] = result
; 00: item is not key item
; 01: item is key item
IsKeyItem::
push hl
push de
Expand Down