QoL: Add Gameplay Option to Disable the Search Spell

[wkdgmr]

Overview
This PR addresses the deprecation and uselessness of the "Search" spell when the Show Item Labels option is enabled. With the new Disable Search Spell gameplay option enabled, our changes ensure that:

• Scrolls, Books, and Staves dropped in the dungeon are prevented from dropping if they would grant the SpellID::Search and instead re-rolled.
• Adria’s shop no longer offers items that provide the Search spell.
• Griswold and Wirt no longer offers items that provide the Search spell.
• The Monk’s class skill is updated from Search to Infravision.
• The Search effect is automatically always enabled when viewing the map.

Rationale
The Search spell has become redundant in DevilutionX due to the implementation of the Show Item Labels feature. Since players can now easily see item information on screen, the utility of the Search spell is diminished, and it has grown to be more of a relic than a useful ability. Lua mods will be able to enhance Show Item Labels in the future as well by adding the squares on the map which is the only thing Search does that Show Item Labels does not (which doesn't really add much value anyway, just a convenience thing).

In our changes, we are updating the Monk’s class Skill by replacing the Search spell with the Infravision spell. This adjustment aligns with the Monk’s thematic identity as described in Hellfire’s materials—a warrior whose inner focus and spiritual insight allow him to see beyond the ordinary, revealing what lies hidden in the darkness. Moreover, Infravision represents a unique skill that mirrors the special abilities of the other classes, whereas besides our intentions to disable Search, it is worth mentioning that Monk is the only class where their skill is learnable as a generic spell and does not reflect the distinct nature of the other classes skills.

Compatibility & Stability

• This PR maintains backwards compatibility. When the Disable Search Spell option is disabled, the Monk will lose access to Infravision and regain the Search skill.
• It ensures that item generation remains stable and items that are generated with this option enabled will not morph.
• The changes occur only when the Disable Search Spell option is enabled, leaving standard behavior intact for users who do not opt in.

Summary of Changes

• Dungeon Item Generation: When the Disable Search Spell option is active, any droppable item that would generate SpellID::Search is re-rolled via SetUpBaseItem(), SpawnItem(), CreateMagicItem()
• Vendor Item Generation: Prevents Vendors from offering items that grant the Search spell when the option is enabled. This is done by adding Checks to SpawnWitch, SpawnBoy, SpawnOnePremium shop logic skipping items generated with Search in a manner that doesn't mess with the index and will result in Stable items being sold.
• Player Initialization: Updates the Monk’s class skill to Infravision in a revertible and backwards compatible manner. Logic was added to handle this seamlessly.
• The Search effect is automatically always enabled when viewing the map.

These changes aim to bring certain obsolete/useless Hellfire elements in line with modern QoL as an optional toggle.

[wkdgmr]

I received some feedback on the item re rolling. My addition of idx re roll in spawnitem is not good, will commit a change to this soon. Need to address Scrolls and their associated spells all being base items and need to re roll this at the idx level

[wkdgmr]

Okay, I am comfortable where this is at now functionally. There is another PR that as submitted after this one effecting the spellbook skill logic that I may have to make some changes for but I would be willing to do that if needed

[wkdgmr]

There were some line changes made by my document formatter to pass the clang check, apparently it removed some brackets. This seems to be a good thing, but I was just formatting at that point to pass clang check that wasn't intentional.

Also there are a few areas of my code that could be deduplicated/slightly refactored to be a bit cleaner, I will push a commit for that soon

[wkdgmr]

The PR is now functionally and technically sound as well as formatted correctly. We may need to force this as a forced on/off by game creator type of thing looking into that

[yuripourre]

@wkdgmr first of all I agree with the motivation but I know this change is very subjective.

The title is misleading, you are not introducing only the ability to disable the Search, you are actually adding a toggle to replace it by infravision and I think that's where this PR starts entering in mod territory.

I am totally fine with a PR that adds an option that only disables the search skill. But replace it by infravision requires a broader discussion and I don't think it would be well received.

As a suggestion, for the infravision part, I think you could try to do this as a mod using lua script.

[wkdgmr]

@wkdgmr first of all I agree with the motivation but I know this change is very subjective.

The title is misleading, you are not introducing only the ability to disable the Search, you are actually adding a toggle to replace it by infravision and I think that's where this PR starts entering in mod territory.

I am totally fine with a PR that adds an option that only disables the search skill. But replace it by infravision requires a broader discussion and I don't think it would be well received.

As a suggestion, for the infravision part, I think you could try to do this as a mod using lua script.

Hi Yurri,

To clarify, we would only be giving the Monk access to Infravision as a class skill when Search is disabled. Search Items will either be skipped, rerolled, or just not dropped at all. Your response made it seem like you thought I am replacing Search wholesale with Infravision, which is not the case apologies if it came across that way.

[yuripourre]

@wkdgmr yes, I understand. Let me try to clarify. You are doing two things in this PR:

1 - Adding a toggle to disable the Search Spell
2 - Giving to monk ability of infravision if toggle (1) is on

I think 1 is reasonable, I think 2 requires a broader discussion.

This line concerns me:

return *GetOptions().Gameplay.disableSearch ? SpellID::Infravision : SpellID::Search;

Maybe you can create two PRs? So 1 and 2 can be discussed separately?

[wkdgmr]

@wkdgmr yes, I understand. Let me try to clarify. You are doing two things in this PR:

1 - Adding a toggle to disable the Search Spell 2 - Giving to monk ability of infravision if toggle (1) is on

I think 1 is reasonable, I think 2 requires a broader discussion.

This line concerns me:

return *GetOptions().Gameplay.disableSearch ? SpellID::Infravision : SpellID::Search;

Maybe you can create two PRs? So 1 and 2 can be discussed separately?

@yuripourre That line there is in one of the areas of the code that governs the skills of each character class. disableSearch toggled on returns the SpellID::Infravision and toggled off it returns what it did originally which was SpellID::Search for the Monk.

I completely understand where you are coming from saying that disabling Search is reasonable but maybe have reservations or questions around updating the Monk skill. This could very easily be two different PR's, but at the same time they do go hand and hand in my mind. As you know, Search is the Monk's class skill. So if we are disabling Search, why would we want the Monk to either A) Still have Search or B) Have no skill at all? I think the spell Infravision slides in here nicely for the Monk, and I think it's important to address the fact that he has the Search spell as his class skill in this same PR about disabling Search.

I don't mind fielding discussions around these two things distinct from each other, but they aren't exactly mutually exclusive. Infravision is perfect for the Hellfire Monk. Search and Infravision icons are very similar looking visually (not really an argument but worth pointing out), and both their functionality fits the Hellfire Monks lore of being able to find things in the darkness. Also Infravision has functional gameplay impact and is interesting... Whereas Search is obsolete and just basically a museum piece for someone who wants the vanilla Hellfire experience. I understand balance changes like this are taboo for the DevX project, and I have no problem just creating a mod with this feature... but to be honest DevX deserves to have something this cool natively within itself. The Monk is the worst class, and Search is the worst spell. This breathes life into some aspects of the game that are delipidated and obsolete. Get search out of the way, and give Monk a skill that lets him compete with the other classes.

[yuripourre]

@wkdgmr yes, I understand. Let me try to clarify. You are doing two things in this PR:
1 - Adding a toggle to disable the Search Spell 2 - Giving to monk ability of infravision if toggle (1) is on
I think 1 is reasonable, I think 2 requires a broader discussion.
This line concerns me:

return *GetOptions().Gameplay.disableSearch ? SpellID::Infravision : SpellID::Search;

Maybe you can create two PRs? So 1 and 2 can be discussed separately?

@yuripourre That line there is in one of the areas of the code that governs the skills of each character class. disableSearch toggled on returns the SpellID::Infravision and toggled off it returns what it did originally which was SpellID::Search for the Monk.

I completely understand where you are coming from saying that disabling Search is reasonable but maybe have reservations or questions around updating the Monk skill. This could very easily be two different PR's, but at the same time they do go hand and hand in my mind. As you know, Search is the Monk's class skill. So if we are disabling Search, why would we want the Monk to either A) Still have Search or B) Have no skill at all? I think the spell Infravision slides in here nicely for the Monk, and I think it's important to address the fact that he has the Search spell as his class skill in this same PR about disabling Search.

I don't mind fielding discussions around these two things distinct from each other, but they aren't exactly mutually exclusive. Infravision is perfect for the Hellfire Monk. Search and Infravision icons are very similar looking visually (not really an argument but worth pointing out), and both their functionality fits the Hellfire Monks lore of being able to find things in the darkness. Also Infravision has functional gameplay impact and is interesting... Whereas Search is obsolete and just basically a museum piece for someone who wants the vanilla Hellfire experience. I understand balance changes like this are taboo for the DevX project, and I have no problem just creating a mod with this feature... but to be honest DevX deserves to have something this cool natively within itself. The Monk is the worst class, and Search is the worst spell. This breathes life into some aspects of the game that are delipidated and obsolete. Get search out of the way, and give Monk a skill that lets him compete with the other classes.

I agree with your motivation, makes a lot of sense. Let's wait and see what others think.

I would be fine with those changes but I think it requires broader discussion.

[wkdgmr]

Spun up a test build for this if people are interesting in seeing it in action
https://github.com/wkdgmrclub/devilutionX/releases/tag/v1.00

[Copilot]

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

Source/items.cpp:1529

• The PR description mentions that staves, in addition to scrolls and books, should be handled when disabling the Search spell. Please verify if staves require similar re-roll logic or additional handling in this branch.

if ((item._iSpell == SpellID::Search) && *GetOptions().Gameplay.disableSearch) {

[wkdgmr]

Updated test build:

https://github.com/wkdgmrclub/devilutionX/releases/tag/v1.01

[wkdgmr]

Test build updated with newest commits
https://github.com/wkdgmrclub/devilutionX/releases/tag/v1.02