Forgottenserver: Loot from monsters does not check if string is correct

[WibbenZ]

Before creating an issue, please ensure:

• [x] This is a bug in the software that resides in this repository, and not a
  support matter (use https://otland.net/forums/support.16/ for support)
• [x] This issue is reproducible without changes to the code in this repository

Steps to reproduce (include any configuration/script required to reproduce)

1. Ex in #2842, TFS does not check (/ report an error) when the item does not exist

Expected behaviour

Throw an error that the item does not exist

Actual behaviour

Nothing

Environment

Does not matter AFAIK

[nekiro]

Arent we going to drop any item name support? I think we should use ids everywhere.

[WibbenZ]

Not sure what the rest think, but if you ask me yes.
But it's not a quick change to move from strings to ids and add a comment saying what the item is named.
Till then this should be fixed, to avoid spelling item names wrong.

[nekiro]

I can probably write some parser that will parse items by name and convert them to ids

[EPuncker]

actually we went from using IDs to using names, now we will go the other way around? xD btw about this bug report itself, I think tfs only check for this if monster is actually spawned, and in my opinion it should check for errors in ALL monsters that are actually registered in the monsters.xml, not only the spawned ones

[DSpeichert]

Dropping support for referring to items by names is probably a no-no. Think about all the data packs that exist with names for items in monsters, etc.

TFS should:
a) warn for items that don't exist
b) warn for items that are non-unique by name when used

But it should still do the most obvious thing.

[Leo32onGIT]

I didn't realise you could use string in the names until recently. Makes for really easy readability from the xml itself. Does need the warnings though.

[EvilHero90]

actually this all is implemented already.


so this can be closed.

EDIT:
revscriptsys monsters are lacking this yet but I'll make a PR ready

[WibbenZ]

Still in this case something is wrong, ex #2842

It does report an error when I changed "dagger" to "daggerr" but ignores the orshabaal loot ("tedy" compared to "teddy"

All monsters should be checked on start-up, ex found another error just now when spawning orshabaal.

Thing like this makes it harder to test new monster PR or custom edits etc
This is the reason why we should avoid string loading items or check if we got more than 1 item with a unique name during start-up.

[EvilHero90]

well it immediatly returns false once it finds an error, so the second error wont even get shown (if that's what you are refering to)
Edit:
The problem with orshabaal is actually the problem I've already adressed in #2798 with the "not loaded" xml monsters on startup

[WibbenZ]

Well either we change it so we print all errors and continue (like now, orshabaal spawns and you can loot items) or we print the error and stop loading the server.
No matter what we can't have hidden errors, if we can't find an item via string / id there should be an error when you start the server.