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

Validate duplicated item_db entries #2649

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Emistry
Copy link
Member

@Emistry Emistry commented Mar 7, 2020

Pull Request Prelude

Changes Proposed

  • Enable user to detect "used" item_db (happen when server has custom item_db that clashed with latest update from Herc if any)
  • able to detect duplicated items where inherit field aren't used.
  • Enable user to detect "duplicated" item_db (happen when user aren't aware of multiple duplicated entries in the past, only happen if inherit: true aren't used.)
  • Encourage user to use inherit: true field for their custom changes to existing item_db.

Issues addressed:
Detect whichever item_db ID are currently clashed with each other (if any)
Lesser duplicated entries of item_db, especially server who has alot custom items.

- encourage usage of `inherit` field.
- able to detect duplicated items where inherit field aren't used.
@Emistry Emistry added type:enhancement Issue describes an enhancement or feature that should be implemented component:databases Affecting the databases available in the db/ folder labels Mar 7, 2020
@MishimaHaruna
Copy link
Member

We already have a duplication check (within the same file), to avoid that the same item id is defined more than once in item_db or item_db2, so I think this is redundant.

There are use cases for non-inheriting entries in the item_db2 (when you want to completely override an item, without inheriting any fields from the main entry). If we don't want to allow that, then we should remove the inherit option, and make the item_db2 always inherit instead of overriding.

The thing I'd really do, instead of forcing people to inherit instead of overriding, is to update the duplicate check in itemdb_readdb_libconfig, to work for item IDs > MAX_ITEMDB (i.e. it could use a dbmap instead of an array)

@MishimaHaruna MishimaHaruna added the codereview:rejected Code review was negative and the pull request can't be accepted label Mar 7, 2020
@Emistry
Copy link
Member Author

Emistry commented Mar 7, 2020

hmm ... if i recall correctly, currently the duplicated items are detected only if they exists within same item_db file.
if i have multiple item_db or i create same item in both item_db and item_db2, it doesn't warn me, it will just simply overwrite the existing item.

but if they want to totally overwrite an existing item, they could still do it, but overwrite all values, but just required the extra inherit field.

@4144
Copy link
Contributor

4144 commented Mar 8, 2020

may be better if need really duplicate, add some special flag into duplicated db entry?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codereview:rejected Code review was negative and the pull request can't be accepted component:databases Affecting the databases available in the db/ folder type:enhancement Issue describes an enhancement or feature that should be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants