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

Simplify scary logic around inventory item buttons. #520

Merged
merged 3 commits into from
Jan 9, 2025

Conversation

NQNStudios
Copy link
Collaborator

The logic that was causing #233 was marked as kludgy and in need of a redo. I've done that, and I think the new logic is more correct.

@CelticMinstrel
Copy link
Member

  1. Does this render ITEMBTN_NORM and ITEMBTN_ALL as entirely unused? If so, they can be deleted.
  2. This doesn't break the ID / Ench / etc buttons, does it?

@NQNStudios
Copy link
Collaborator Author

I don't think so, but it needs testing. Incidentally, how would I make a party with an unidentified item? Edit a scenario and have them pick it up? Does the PC editor allow adding items? Should I just play the game until I find some? I do need to keep just testing VoDT so maybe that's what I'll do.

@CelticMinstrel
Copy link
Member

The PC editor and the in-game debug mode both support adding items, but if I recall correctly, they automatically identify the items added. Probably the best way to get unidentified items is to enter a scenario and steal some stuff. (Picking stuff up off the ground is fine too, but those kinds of items often have the "Always Identified" flag.)

@NQNStudios
Copy link
Collaborator Author

NQNStudios commented Dec 12, 2024

  • test can use items in combat, but not while targeting
  • test doesn't break ID/Enchant/etc buttons
  • see about removing ITEMBTN_NORM and ITEMBTN_ALL

@CelticMinstrel
Copy link
Member

Just a note: the full list of buttons for point 2 is identify (ID), sell, enchant (Ench), and recharge (Inc).

@NQNStudios
Copy link
Collaborator Author

@CelticMinstrel I tested all those cases, I think it's good to go.

@CelticMinstrel CelticMinstrel merged commit b7da0f0 into calref:master Jan 9, 2025
3 of 6 checks passed
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