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

Added POKEMART_LIST_END to avoid users accidentally removing it #1947

Merged
merged 3 commits into from
Dec 4, 2024

Conversation

AsparagusEduardo
Copy link
Contributor

Description

Previously, because it had the ITEM_/DECOR_ prefix, users would mistakenly remove it from their custom mart lists.
This should hopefully aleviate the issue.

Discord contact info

AsparagusEduardo

@GriffinRichards
Copy link
Member

GriffinRichards commented Nov 8, 2023

I think this could be much more useful not as an alias of ITEM_NONE, given this isn't that much more visually distinct from the end of the list, and it's very unlikely anyone would redefine it. It might be more useful to define something like this:

.macro POKEMART_LIST_END
    .2byte ITEM_NONE @ (or generically .2byte 0)
    release
    end
.endm

So that A. it would be more visually distinct from the end of the list:

    .2byte ITEM_CARBOS
    .2byte ITEM_HP_UP
    POKEMART_LIST_END

and B. so the unnecessary release and end are hidden and can be removed by editing one location.

@AsparagusEduardo
Copy link
Contributor Author

Good idea! Since it's gonna be a macro, would pokemartlistend be a better name for it?

@GriffinRichards
Copy link
Member

Since it's gonna be a macro, would pokemartlistend be a better name for it?

Up to you

@AsparagusEduardo
Copy link
Contributor Author

13-month PR comeback babyyy!

@GriffinRichards GriffinRichards merged commit 3f98c78 into pret:master Dec 4, 2024
2 checks passed
@AsparagusEduardo AsparagusEduardo deleted the pret/pr2/martListEnd branch December 4, 2024 22:41
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