forgottenserver icon indicating copy to clipboard operation
forgottenserver copied to clipboard

Improvements that the Bestiary needs.

Open MillhioreBT opened this issue 2 years ago • 11 comments

Pull Request Prelude

  • [x] I have followed [proper The Forgotten Server code styling][code].
  • [x] I have read and understood the [contribution guidelines][cont] before making this PR.
  • [x] 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!

MillhioreBT avatar Apr 20 '23 04:04 MillhioreBT

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)

ArturKnopik avatar Apr 21 '23 13:04 ArturKnopik

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. image

MillhioreBT avatar Apr 21 '23 14:04 MillhioreBT

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.

soul4soul avatar Apr 23 '23 17:04 soul4soul

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...

MillhioreBT avatar Apr 25 '23 08:04 MillhioreBT

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?

ranisalt avatar Apr 25 '23 10:04 ranisalt

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 https://github.com/soul4soul/ot-monster-converter/pull/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.

soul4soul avatar Apr 29 '23 12:04 soul4soul

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.

omarcopires avatar May 15 '23 16:05 omarcopires

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

EPuncker avatar May 15 '23 19:05 EPuncker

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.

soul4soul avatar May 21 '23 18:05 soul4soul

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.

It is due to this kind of selfish behavior that people distance themselves from TFS. These comments only hinder development, adding nothing except for blocking the entire request and causing the developer to abandon it.

It would be easy to resolve if you simply submitted your version of the code and let the team discuss which version should be merged. Instead, you just block something that would be useful for the community by saying, "I won't reiterate my reasons I already mentioned above."

omarcopires avatar May 21 '23 23:05 omarcopires

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.

It is due to this kind of selfish behavior that people distance themselves from TFS. These comments only hinder development, adding nothing except for blocking the entire request and causing the developer to abandon it.

It would be easy to resolve if you simply submitted your version of the code and let the team discuss which version should be merged. Instead, you just block something that would be useful for the community by saying, "I won't reiterate my reasons I already mentioned above."

Having a difference of opinions doesn't make behavior selfish. I'm not part of TFS development team they are free to merge this PR. I do try and contribute to the community and I'm sharing that by going this route it will detract from the features I will be able to support with my tool. Albeit my tool has few users and most are one times users.

My previous comment was only to rebut your argument that my suggestions requires 800 files to be edited as not being a good one. Both approaches requires 800 files to be manipulate. As for not relisting my reasons, why would I do that? They are one the same page a few comments up, I assume those who needed to read them have done so already.

soul4soul avatar May 27 '23 19:05 soul4soul