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

Game.getMonsterTypes() #2798

Merged
merged 2 commits into from Mar 11, 2020
Merged

Game.getMonsterTypes() #2798

merged 2 commits into from Mar 11, 2020

Conversation

EvilHero90
Copy link
Contributor

This will add the function Game.getMonsterTypes()
It will return as the key the monster name and as the value the metatable.

This helps a lot if you want to register a certain event to all of your existing MonsterTypes.

Beware!
This does not trigger unloaded MonsterTypes which are created through xml and not once spawned on map load, it'll only fetch MonsterTypes which are created through revscriptsys (monster/lua) or xml monsters which are load and spawned on startup.

This will add the function
Game.getMonsterTypes()
It will return as the key the monster name and as the value the metatable.
forgot to remove an unused variable
@WibbenZ
Copy link
Member

WibbenZ commented Feb 25, 2020

It should either return all monster types or change the function name so it's clear that it will only return rev monster types.

@EvilHero90
Copy link
Contributor Author

It should either return all monster types or change the function name so it's clear that it will only return rev monster types.

Well it basicly returns all monster types (from lua and xml), the problem is the following:
if you have let's say the monster "test.xml" added to your server but you don't let it spawn somewhere on your map for once on server load, then this monster is marked as "unused" and is in a different map::list which doesn't even hold the data of the monster type itself as it has not been created yet (because it's not needed) this prevents the server to create monster types which the server primarly doesn't use.
Monster types through revscriptsys are immediatly load and ready to go that's why it doesn't apply to them.
We could either get rid of the entire "unused part" or we keep it as is.

@EPuncker
Copy link
Contributor

the unused part is okay IMO, since there are creatures that we only spawn during raids/events

@EvilHero90
Copy link
Contributor Author

LGTM

@@ -241,6 +241,7 @@ class Monsters
bool deserializeSpell(MonsterSpell* spell, spellBlock_t& sb, const std::string& description = "");

std::unique_ptr<LuaScriptInterface> scriptInterface;
std::map<std::string, MonsterType> monsters;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of moving that from being private, maybe we should add public method to acquire that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't really feel the necessity to do so, as we just use this one time.
If we had several cases where we would need the public class, then yes it should be done but other than adding one more line it wont really gives us any kind of benefit (atleast that's what I think)

Copy link
Member

@Znote Znote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great!
image

@Znote Znote merged commit 19b4343 into otland:master Mar 11, 2020
@Znote
Copy link
Member

Znote commented Mar 11, 2020

@EvilHero90 EvilHero90 deleted the game-monstertypes branch March 11, 2020 20:16
@soul4soul
Copy link
Contributor

@Znote
Nice use of wsl 👌

@Znote
Copy link
Member

Znote commented Mar 11, 2020

yep, got the ultimate development & git reviewing environment set up now, where I use the best of both Windows and Linux. I have also made a script that lets me checkout, compile and run other forks with a simple shell command. :)

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.

None yet

6 participants