Skip to content

Conversation

@Eebit
Copy link
Collaborator

@Eebit Eebit commented May 4, 2025

This PR makes the following changes:

  • Updates the event scripts to use the constants/items.h enum for item-related values
  • Corrects the subcommand names and macros based on StanHash/DOC - EventDoc/EventCodes.md
    • GIVEITEMTO -> GIVE_ITEM
    • GIVEITEMTOMAIN -> GIVE_MONEY
    • GIVETOSLOT3 -> TAKE_MONEY
  • Adds additional helper macros GiveItemTo(pid, itemId) and GiveMoney(amt)

Copy link
Collaborator

@MokhaLeee MokhaLeee left a comment

Choose a reason for hiding this comment

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

I don't oppose fixing incorrect EA instructions, but after all, these instructions are built-in functions of EA. Directly modifying the original definitions would cause discrepancies between EA scripts and decompiled event scripts. Therefore, I recommend append the defination rather than directly repalce it. For example, append a definition as #define GIVE_ITEM GIVEITEMTO at the end of this file.

@MokhaLeee
Copy link
Collaborator

but after all, these instructions are built-in functions of EA
https://github.com/FireEmblemUniverse/SkillSystem_FE8/blob/master/EventAssembler/Event_assembler_language.txt#L1256

@Eebit
Copy link
Collaborator Author

Eebit commented May 13, 2025

I don't oppose fixing incorrect EA instructions, but after all, these instructions are built-in functions of EA. Directly modifying the original definitions would cause discrepancies between EA scripts and decompiled event scripts. Therefore, I recommend append the defination rather than directly repalce it. For example, append a definition as #define GIVE_ITEM GIVEITEMTO at the end of this file.

It's a fair point -- I took a slightly different approach and added the backwards-compatible macros to EA_Standard_Library/Code_Aliases.h in dc6e9ef. I'm hoping this is an acceptable compromise, but if you'd prefer to preserve carefully the main EA definitions I can do it the way you suggested!

@CT075
Copy link
Member

CT075 commented May 13, 2025

We could do the reverse, and change the name in EA; I think that's better than preserving a wrong name, especially GIVEITEMTOMAIN.

CALL(EventScr_RemoveBGIfNeeded)
SVAL(EVT_SLOT_3, 0x1388)
GIVEITEMTOMAIN(CHAR_EVT_PLAYER_LEADER)
GiveMoney(5000)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like here are typoes as it is all CAPITAL letters definition in header?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, my fault, it is okay here.

@MokhaLeee
Copy link
Collaborator

I have no objection to the current plan. You guys can discuss this further as Cam propoesed new idea.

@CT075
Copy link
Member

CT075 commented May 13, 2025

For what it's worth, as a matter of terminology, the EA commands are not "built-in" in the sense that the phrase is normally used; EA Language.txt is generated from the Language Raws here. The Language raws in general are horrendously out-of-date (like, "predates version 1 of the original skill system" out of date) and I would be in favor of pushing to update those in general, especially now that we have a better understanding (through this project) of what these commands actually do.

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.

3 participants