Skip to content

Conversation

@WheteThunger
Copy link
Contributor

  • Added hooks from BaseRidableAnimal to RidableHorse2, including OnHorseLead, CanLootEntity, and OnRidableAnimalClaim[ed]
  • Moved existing Horse hooks to the recently added Animal category
  • Renamed existing Horse hooks to have the [BaseRidableAnimal] tag for clarity now that there are two versions

I tested all new hooks.

@IIIaKa
Copy link
Contributor

IIIaKa commented Mar 3, 2025

@WheteThunger
Hello, such a PR already exists.
#551

@WheteThunger
Copy link
Contributor Author

I see, thanks for pointing it out. That PR appears to focus on adding new hooks, while this PR focuses on solely establishing parity between BaseRidableAnimal and RidableHorse2. Merging this PR will restore functionality in multiple of my plugins without requiring me to make any changes to them, which is ideal. Glancing at your PR, I see multiple problems. If you can correct those problems, I will close this PR.

  • Renaming a hook (OnHorseLead -> OnRidableAnimalLead) is a backwards-incompatible change
  • Removing hooks for BaseRidableAnimal is a backwards-incompatible change
  • Missing CanLootEntity for RidableHorse2; I see OnTryLootEntity but this is not the name used in BaseRidableAnimal or other classes

@IIIaKa
Copy link
Contributor

IIIaKa commented Mar 3, 2025

  • The BaseRidableAnimal and RidableHorse classes are deprecated, making it unnecessary to keep changes in them;
  • The change from OnHorseLead to OnRidableAnimalLead was made to maintain a consistent naming style(OnRidableAnimalClaim, OnRidableAnimalClaimed, OnRidableAnimalTow, OnRidableAnimalDetach and etc.).
  • The change from CanLootEntity to OnTryLootEntity was made because this hook is object, not boolean; Can hooks are only for boolean.

Regarding the changes in naming: RidableHorse no longer exists(unless spawned manually), and all plugins will have to update the type from RidableHorse to RidableHorse2. Therefore, it makes sense to also update the names to more appropriate ones that align with the overall concept.

@WheteThunger
Copy link
Contributor Author

Although the classes are deprecated, I'm not aware of an estimated removal date. If you know it, please share. FP has deprecated things before but has taken their time to remove them. For example, pooling methods, which FP claimed would be removed one month later, but they are still here many months later. I'm not suggesting that anyone develop new plugins or features with the old horses, but existing plugins should be taken into account.

The point of deprecation is to give users time to adjust to the changes before feature removal. Existing pligins should not be broken at random by Oxide maintainers because of an upcoming removal. Assuming that every plugin can react immediately to backwards-incompatible changes suggests a misunderstanding of the plugin ecosystem. Many plugins are not maintained, some were merely authored in a forum post, some were written by Gen AI, etc. I understand that the prefab change for the old horses immediately broke all plugins that relied on knowledge of the prefab, but it was an easy fix (find/replace) even for non-coders, and some plugins did not even need code changes due to the prefab being a config option that the server owner could change. You're right that the old horses no longer spawn naturally in Rust, but some plugins still spawn them, and other plugins need to continue to work with them.

The new horses work differently and some plugins have had to undergo significant changes to restore existing capabilities, so it will take time for the community to upgrade most plugins to support them. It's essential that the community be given sufficient time to make the necessary upgrades. While we are we are beholden to when FP actually removes the old horses, we should not make the problem worse by unnecessarily accelerating that timeline.

Regarding Can vs On hooks, I agree that "new" hooks should follow the convention you described, but I argue this shouldn't be thought of as a new hook. This is backfilling a hook to another class, so it should match the conventions used in other classes.

@IIIaKa
Copy link
Contributor

IIIaKa commented Mar 4, 2025

@WheteThunger
Hello. Well, I think even FP doesn’t know when they'll remove outdated methods and classes :)
The old horses simply don’t spawn anymore unless manually spawned via a command or plugin. But I don’t think anyone would seriously use them, as the new horses are better, making the old ones obsolete.
As for existing plugins, they have already "stopped working" since the old horses no longer exist. Any hooks related to them just don’t trigger, even if this hooks still exist. In this case, it might be worth keeping modifications to public methods to prevent compilation errors, but leaving the hooks makes no sense. If this isn't addressed soon, they’ll be forgotten and remain "hanging" indefinitely.

#545
In fact, there are a bunch of hooks with incorrect names. I reviewed all of them while trying to implement conflict avoidance for object hooks. You're right in saying that plugins need time to "adapt", but while we wait for old plugins to "adapt", new plugins are being created daily, increasing the number of plugins using incorrect names. This can be seen in how long it takes for plugins to be approved on Lone, Codefling and uMod. On uMod, my plugin has been sitting with a '0% Completed' status for three months already.

Judging by the time that has passed, incorrect hook conflicts have been around for many years and there are no visible signs of progress so far. That's why it's worth at least gradually moving towards solving this problem, starting with the names. For example, new hooks could be added while marking the incorrectly named ones as deprecated, giving developers 2-3 months to transition.

That’s just my opinion, it may be wrong.

@WheteThunger
Copy link
Contributor Author

WheteThunger commented Mar 4, 2025

Not all existing plugins stopped working. Hooks continue to be called for the old horses. Plugins still referencing the RidableHorse and BaseRidableAnimal will receive the hook calls.

Plugins that explicitly reference the prefab path, such as plugins which spawn horses, were impacted, but as I explained in my previous post, it was a trivial update that even non-coders can do, and I have found that many server owners will take it upon themselves to figure out such changes (often guided by other users in various Discord servers). As I was also saying, some plugins exposed the prefab path as a config option, which allows server owners to correct the problem without a code change.

The Oxide team has renamed hooks before, and what we found is that it's a collective waste of community time. Hundreds of server owners will react to the deprecation notices, resulting in hundreds of independent discussions across various forums (multiple websites, multiple discord servers, many DMs). It simply distracts server owners and developers from spending their time on making truly meaningful changes. The benefit of renaming hooks is very negligible as developers can simply read docs or read the assemblies to learn how to correctly use a hook (which they should do anyway if they are adopting a new hook). Renaming hooks also has a way of forcing old or unmaintained plugins to break, which shouldn't be necessary if those plugins work. Making backwards incompatible changes sometimes needs to be done to enable forward progress, but renaming hooks is simply NOT one of those cases.

@IIIaKa
Copy link
Contributor

IIIaKa commented Mar 5, 2025

Everyone has their own opinion, but I don't think that I, you, or Oxide should be responsible for plugins that are no longer supported. The focus should be on ensuring that existing and future plugins work as intended.

For example, when working with the TimedPermissions plugin, I was developing a plugin that needed to integrate with it, but I lacked a couple of API methods/hooks. I wanted to request their addition, but then I saw that the plugin had been abandoned for three years. As a result, I simply decided to create my own alternative with more features for both developers and server owners. You could say that I could have opened a support ticket or a patch request to add the missing hooks/methods. But I've already done that with 2-3 other plugins, and there's still no response for over a year...

https://codefling.com/plugins/temporary-permissions
https://umod.org/plugins/bN5MQwO510 - This plugin has actually been on your website for more than 3 months without review.

What I want to say is that, no matter how much you worry about plugins abandoned by their authors, their relevance eventually fades. Some plugins simply stop working with such plugins because there is no support from their developers. Meanwhile, new alternatives emerge with active authors who continue to maintain and improve them.

And with names, when if not now is the right time to switch to new appropriate names? For RidableHorse2, corrected names can be applied, because anyone updating their plugin to replace RidableHorse or BaseRidableAnimal with RidableHorse2 will be updating their plugin anyway, and at the same time, they can also rename the hooks.

@bmgjet
Copy link
Contributor

bmgjet commented Mar 5, 2025

My 2c, Don't rename hooks. I don't want to have to waste time updating stuff when I don't have to.
The name of the hook means basically nothing to me since I'm looking at the decompiled code and see what hooks I want to use in there. As every one should be doing since all the documentation in the world isn't as straight forward as looking at the code and seeing how a hook works. It could be called GenericHook1 for all I care as long as it works and isn't going to be changed unless it really has to change because some code breaking changes.

@IIIaKa
Copy link
Contributor

IIIaKa commented Mar 5, 2025

I see, it's sad that everyone thinks only of themselves...

Don't misleadingly named hooks confuse you? For example, when I see a hook that starts with 'Can', I assume it's a boolean hook, but that's not always the case due to incorrect naming. As a result, for 'Can' hooks, I have to check the source code every time.
If the names were correct, there would be no need to look at the source code, it would all be clear right away. That's the whole point of properly naming hooks, to make it immediately clear where the hook is called and what can be done with it.

@rostov114
Copy link
Contributor

You can't just rename hooks, you need to mark them as deprecated (src/RustHooks.cs) and give people time to adapt them.
Besides, uMod has quite a few plugins that aren't unmaintained, will you adapt them all to the new hooks?
The idea is good, correct. But there are already few people in the community, and you propose to add a bunch of work to those who remain.

@MrBlue MrBlue mentioned this pull request Mar 5, 2025
@MrBlue MrBlue merged commit 2b70b54 into OxideMod:develop Mar 5, 2025
@MrBlue
Copy link
Member

MrBlue commented Mar 5, 2025

Thanks for the pr!

@WheteThunger WheteThunger deleted the ridable-horse-2-hooks branch March 7, 2025 03:24
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.

5 participants