Skip to content

Feature/auto sell weapon#67

Open
TiMMyyMMiT wants to merge 10 commits intoShiningForceCentral:feature/sell_before_buyfrom
TiMMyyMMiT:feature/auto_sell_weapon
Open

Feature/auto sell weapon#67
TiMMyyMMiT wants to merge 10 commits intoShiningForceCentral:feature/sell_before_buyfrom
TiMMyyMMiT:feature/auto_sell_weapon

Conversation

@TiMMyyMMiT
Copy link
Contributor

🎯 Purpose of This PR

Adjusts the weapon purchase flow so that player is given the option to sell currently equipped weapon before purchasing the new one. This allows purchase of weapons even when all 4 item slots are filled (assuming that player does choose to sell the existing weapon).
NOTE: Intentionally did not affect the Blacksmith flow.

Fusion_HWZKJoxZJp

🧩 Summary of Changes

List the key changes made in this PR. For example:

  • Added 1 new line of dialog in an empty slot (dialog 406 (hex 0196), text line 407)
  • Added patch to af2patches.asm
  • Majority of the changes are in shopcations.asm
    Features and changes: (test these conditions)
  1. All calls/changes are wrapped so that they do not affect non-standard builds or builds without this patch
  2. First checks if item is a weapon and is equippable to current character. If 'no' then jumps to the default flow*
    • NOTE: The default flow checks these things again (to determine if should equip and if hands are full)
  3. Then checks if character already has a weapon equipped (ignore rings). If 'no' weapon is equipped then jump to regular flow*
  4. Then shows new dialog to say that you can sell your equipped weapon. If 'no' then just continues the regular flow*
    • If 'yes', code with set a sentinel value then jump to the sell item flow. Once sell item flow is completed (sold or not) then returns to the regular flow*
  5. NOTE: Several branches needed to be changed from .b to .w, since extra code was inserted inbetween. These are wrapped to not affect the non-standard build (the basic build).

*"Regular flow" means checking if characters item slots are full, checking if weapon can be equipped, purchasing the weapon, etc.
Tapping into existing game logic saved a lot of duplicate code and minimised chances for bugs.

🔄 Coordination Notes

Questions to ask about publishing a patch:

  1. Should I have built the textbanks?
    • I could see that textbanks aren't changed on Standard branch so I assume that player is expected to build them to get any updated dialog
  2. Is there a best practice for naming the branch points (e.g. "loc_2015E")?
    • I just used a shorthand that reflects the patch "loc_SBB01"-"loc_SBB03"
  3. Flow acts as intended but gets a little convoluted when cursed weapons are equipped. Should the flow stay or should the initial check also check if weapon is cursed?

🧪 How Has This Been Tested?

Please confirm that the following standard tests have been performed. Then, list any additional tests that you ran to verify your changes and provide instructions so we can reproduce them.

  • Run build.bat and confirm that it produces a bit-perfect replica of original rom
  • Run buildstandard.bat or buildexpanded.bat, if applicable, and confirm that build succeeds

I tried to test thoroughly

✅ Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

📚 Additional Notes

My first patch 😁

@TiMMyyMMiT
Copy link
Contributor Author

If this is good then this branch will also need to be merged into standard branch

@wizardwhosaysni
Copy link
Contributor

wizardwhosaysni commented Nov 9, 2025

Hey Timmy,

As you already guessed, Xeno will be more knowledgeable than me on helping you with patch development, but I wanted to say congrats for tackling 68K ASM hacking with a great QoL feature !

While I'm here, I just saw your emulator was Kega Fusion so I'll copy Xeno's emote for a laugh : image
No big deal really :D but just so you know : Kega Fusion is pretty laxist and doesn't help in ensuring that our custom ROM builds are really valid. This is probably not the only issue but there's definitely one that I have in mind : odd addresses. Kega Fusion tolerates the use of odd addresses, while it shouldn't be the case : there should be an address error interrupt call.
Odd address happen quite easily with our custom builds because not all data assets start with an "align 2" directive to ensure they start on an even address.
This is most probably irrelevant in the context of this PR but I still felt the need to share this once.
Looks like the recommended emulator is BizHawk now, but I think quite a few others remain valid for debugging purposes. I've used Regen Debug and Exodus most of the time.

To give a first level of answers to your 3 questions :

  1. No, you aren't supposed to provide pre-built textbanks, as these binary assets are usually not uploaded on the repository, and merging with other patches requires to keep changes at text file level.
    But this is raising an interesting question for all patches relying on updated or new text lines. A possible target could be to add a command line interface to TextManager and make it produce textbanks from buildstandard.bat. Meanwhile, your patch requires manual production of new textbanks indeed.

  2. New labels can totally scrap IDA Pro's initial naming convention, so in my opinion there's no need to use "loc_" prefixes.
    In your context, your new labels can probably be local labels starting with an "@" character. There was no local label in the initial disassembly, so all instances of local labels have been formatted by Xeno and they probably are the best examples you can follow now in terms of naming convention.

  3. The patch can always be improved at any moment later if the flow is indeed too cumbersome for cursed weapons, but no need to think too much about best flow for now in my opinion. This is already providing great comfort for the vast majority of weapon purchases for sure :D

@TiMMyyMMiT
Copy link
Contributor Author

Hey Wiz. I use Fusion and BizHawk. Fusion loads much faster so it better for quick iterations (and I have save states for each point in the game). BizHawk is what I use if I need to test anything properly. I will check out those other emulators too.
On the note of odd addresses: I believe that the compiler throws errors about this so builds shouldn't be possible (but I could be wrong).

@TiMMyyMMiT
Copy link
Contributor Author

@xenometal Got time to approve and merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants