osrsbox / osrsbox-db

A complete and up-to-date database of Old School Runescape (OSRS) items, monsters and prayers
https://www.osrsbox.com/projects/osrsbox-db/
GNU General Public License v3.0
223 stars 80 forks source link

Fixed monster id conflicts in data/monsters/monsters-wiki-page-text-processed.json #215

Closed BrunoC-L closed 3 years ago

BrunoC-L commented 3 years ago

Fixed monster id conflicts in data/monsters/monsters-wiki-page-text-processed.json

that file is in gitignore

did not manage to run the script to update /docs/monsters-json files where I originally saw the problem of molanisk being wrongfully merged with undead zealot.

Assuming the script generating those individual files uses data/monsters/monsters-wiki-page-text-processed.json, that should be fixed.

Resulting line in processed.json, molanisk is now here

Resulting processed.json, undead zealot now has the right id

There is still a problem, undead zealots have ids 10591 & 10592, feel free to reject the PR based on that fact, 10592 is missing from the processed JSON, not all duplicte IDS are missing though, there is a triple try catch when finding the ID and only those that reach the 3rd try will use my "patch", leading to this problem, I think it's a smaller issue than previously though, I'm not sure what most users expect when using the JSON files but I think avoiding conflicts with IDs is pretty good.

osrsbox commented 3 years ago

Thanks for submitting this PR - got me motivated to look into this problem. Unfortunately, got sent down a rabbit hole for a few hours trying to get a solution. The PR fixes one small issue - but the deeper I dug - the more small inconsistencies I found!

The primary problem with to Molanisk was that the id of 1 was being overwritten with the Undead Zealot as the version property had an integer, not the usual name. You're PR does fix this. The Zealot infobox snippet is below:

|version1=1
|version2=2
...
|id = 10591
|id2 = 10592

The second problem was there is not usually a non-incremented id - I think this is one of the only monsters, and a couple items, that has it. So id instead of id1. The Abyssal Demon example below shows the usual method the wiki uses. So id1, not just id:

|id1 = 415,416
|id2 = 7241

So the fix ended up being to better handle the non-incremented id value - which in turn - fixed the other problems. It also seemed to get better data for about 4 monsters, and 10 items. So a success! Anyway - after I retest everything and make sure nothing is broken - will push an update to fix this. Probably won't accept the PR - as I rewrote a lot of the logic in the wiki infobox parser module. But a huge thanks for trying to find a solution to this - and getting me motivated!

BrunoC-L commented 3 years ago

Thanks alot for the explanation!