Improvements that the Bestiary needs. #4412

MillhioreBT · 2023-04-20T04:58:45Z

Pull Request Prelude

I have followed [proper The Forgotten Server code styling][code].
I have read and understood the [contribution guidelines][cont] before making this PR.
I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

These changes improve the readability of the code and raceId has been moved to a better place.
now the occurrence must be set with a name and not with a raw number. (common, uncommon ect...)
Thanks to @ranisalt and @joseluis2g for all the information and suggestions.

Issues addressed: Nothing!

ArturKnopik · 2023-04-21T13:01:12Z

maybe let's leave the hybrid option, if there are no blocks responsible for setting kills, let's set it by default depending on the number of stars, if this data is present, then instead of the default ones, let's use these
this will allow us to generate custom bestiaries records(example: 1000-5000-15000 kills)

MillhioreBT · 2023-04-21T14:00:45Z

maybe let's leave the hybrid option, if there are no blocks responsible for setting kills, let's set it by default depending on the number of stars, if this data is present, then instead of the default ones, let's use these this will allow us to generate custom bestiaries records(example: 1000-5000-15000 kills)

It's very easy to create a new table based on names or raceId or whatever you want, and hook it up to the getBestiaryKills method for custom behavior.

ranisalt · 2023-04-22T19:09:39Z

data/lib/core/bestiary.lua

+	[3] = {kills = {50, 500, 1000}, charmPoints = 25},
+	[4] = {kills = {100, 1000, 2500}, charmPoints = 50},
+	[5] = {kills = {200, 2000, 5000}, charmPoints = 100}
+}


What if I want to create a monster with a custom amount of kills per step?

I'm OK with defaulting to Tibia behavior, but not as much with only being able to have Tibia behavior.

I don't know if the one I posted above is enough.

soul4soul · 2023-04-23T17:48:18Z

I understand the value in having a table with bestiary data because there are only a few combination of values but personally I'd rather all bestiary data live directly in the creature files as it is before the PR. On top of that I have additional bias to have it in the creature files to make it easier for my monster converter to work.

MillhioreBT · 2023-04-25T08:10:17Z

I understand the value in having a table with bestiary data because there are only a few combination of values but personally I'd rather all bestiary data live directly in the creature files as it is before the PR. On top of that I have additional bias to have it in the creature files to make it easier for my monster converter to work.

I don't know who in their right mind would customize over 800 monster files XD. That's what the table is for, so that only 4 lines can control those 800 files.

But to be honest, I'm not sure if it's worth having these configurations in each file, or if it's better to use Lua for this. There are many ways to get monsters with customized kills.

I hope other people can suggest or give ideas...

ranisalt · 2023-04-25T10:54:48Z

On top of that I have additional bias to have it in the creature files to make it easier for my monster converter to work.

How does it work with monsters created in Lua?

soul4soul · 2023-04-29T12:48:22Z

On top of that I have additional bias to have it in the creature files to make it easier for my monster converter to work.

How does it work with monsters created in Lua?

I created lua binding that match TFS monsters revsctiptsys soul4soul/ot-monster-converter#84. For all the formats I support the converter is generally only able to look directly at the monster files not any of the supporting files.

I understand the value in having a table with bestiary data because there are only a few combination of values but personally I'd rather all bestiary data live directly in the creature files as it is before the PR. On top of that I have additional bias to have it in the creature files to make it easier for my monster converter to work.

I don't know who in their right mind would customize over 800 monster files XD. That's what the table is for, so that only 4 lines can control those 800 files.

But to be honest, I'm not sure if it's worth having these configurations in each file, or if it's better to use Lua for this. There are many ways to get monsters with customized kills.

I hope other people can suggest or give ideas...

True, it will be rare that an OT owner will customize each monster. I suspect people will be less likely to touch the default monsters and instead have custom values for their custom creatures. Still, as of now all data about a creature is defined directly in the creature file I'd prefer to keep it that way.

omarcopires · 2023-05-15T16:52:09Z

I agree with what @MillhioreBT says, it doesn't make sense to manipulate more than 800 files to configure 4 or 5 different behaviors if we can simply customize some desired monster just by adding a few extra lines to the code. Maybe just leaving an example script will be enough.

As for the changes to this pull request, everything works as it should and unless someone has any suggestions or code hints it's ready to be merged.

EPuncker · 2023-05-15T19:02:25Z

yeah please let push that in, there is no need to delay things more

soul4soul · 2023-05-21T18:42:07Z

I agree with what @MillhioreBT says, it doesn't make sense to manipulate more than 800 files to configure 4 or 5 different behaviors if we can simply customize some desired monster just by adding a few extra lines to the code. Maybe just leaving an example script will be enough.

As for the changes to this pull request, everything works as it should and unless someone has any suggestions or code hints it's ready to be merged.

Either way you need to touch 800 files to add the difficulty and occurrence. Another PR that was opened already had to work done, making the changes isn't a problem. I won't reiterate my reasons I already mentioned above.

ranisalt · 2023-06-01T09:25:07Z

src/monsters.h

@@ -97,7 +97,11 @@ struct BestiaryInfo
 	std::string className = "";
 	uint8_t difficulty = 0;
 	uint8_t occurrence = 0;
+	std::tuple<uint32_t, uint32_t, uint32_t> totalKills;


Do you see any advantages on using a tuple vs three separate uint32_ts?

I don't think it has any advantage other than looking prettier 😊

MillhioreBT added 2 commits April 20, 2023 00:48

significant improvements to the bestiary system

5d8a09e

Missing revscript monster ocurrence config

d699d2d

MillhioreBT requested review from ranisalt and EPuncker April 20, 2023 04:58

EPuncker added the enhancement Increase or improvement in quality, value, or extent label Apr 20, 2023

MillhioreBT added 3 commits April 20, 2023 17:14

missing dofile bestiary.lua

1b16b3e

fix get race id in kills

22b8054

fix get race id in m types

2bdb197

omarcopires previously approved these changes Apr 20, 2023

View reviewed changes

EPuncker previously approved these changes Apr 20, 2023

View reviewed changes

EPuncker requested review from DSpeichert and nekiro April 22, 2023 00:40

MillhioreBT mentioned this pull request Apr 22, 2023

feature: bestiary tracker #4418

Open

3 tasks

ranisalt reviewed Apr 22, 2023

View reviewed changes

This comment was marked as off-topic.

Sign in to view

otland locked as too heated and limited conversation to collaborators May 28, 2023

MillhioreBT added 2 commits May 31, 2023 22:48

Merge remote-tracking branch 'upstream/master' into bestiary_inp

07bfb9c

suggestions applied

3a7747a

MillhioreBT dismissed stale reviews from EPuncker and omarcopires via 3a7747a June 1, 2023 03:02

EPuncker requested review from ranisalt, omarcopires and EPuncker June 1, 2023 04:35

EPuncker approved these changes Jun 1, 2023

View reviewed changes

ranisalt reviewed Jun 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements that the Bestiary needs. #4412

Improvements that the Bestiary needs. #4412

MillhioreBT commented Apr 20, 2023 •

edited

Loading

ArturKnopik commented Apr 21, 2023

MillhioreBT commented Apr 21, 2023

ranisalt Apr 22, 2023 •

edited

Loading

MillhioreBT Apr 22, 2023

soul4soul commented Apr 23, 2023 •

edited

Loading

MillhioreBT commented Apr 25, 2023

ranisalt commented Apr 25, 2023

soul4soul commented Apr 29, 2023

omarcopires commented May 15, 2023

EPuncker commented May 15, 2023

soul4soul commented May 21, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

ranisalt Jun 1, 2023

MillhioreBT Jun 7, 2023

Improvements that the Bestiary needs. #4412

Are you sure you want to change the base?

Improvements that the Bestiary needs. #4412

Conversation

MillhioreBT commented Apr 20, 2023 • edited Loading

Pull Request Prelude

Changes Proposed

ArturKnopik commented Apr 21, 2023

MillhioreBT commented Apr 21, 2023

ranisalt Apr 22, 2023 • edited Loading

Choose a reason for hiding this comment

MillhioreBT Apr 22, 2023

Choose a reason for hiding this comment

soul4soul commented Apr 23, 2023 • edited Loading

MillhioreBT commented Apr 25, 2023

ranisalt commented Apr 25, 2023

soul4soul commented Apr 29, 2023

omarcopires commented May 15, 2023

EPuncker commented May 15, 2023

soul4soul commented May 21, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

ranisalt Jun 1, 2023

Choose a reason for hiding this comment

MillhioreBT Jun 7, 2023

Choose a reason for hiding this comment

MillhioreBT commented Apr 20, 2023 •

edited

Loading

ranisalt Apr 22, 2023 •

edited

Loading

soul4soul commented Apr 23, 2023 •

edited

Loading