Skip to content
This repository has been archived by the owner on Sep 27, 2023. It is now read-only.

refactor(core): Move set freeday colors to core #86

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ZakharYA
Copy link

Думаю правильнее избавиться от установки цветов и convarов FreeDay'а в отдельном плагине, когда в ядре у нас есть основная функция для установки FreeDay.
С текущей реализацией, если мы хотим выдавать игрокам FreeDay, то нам надо тянуть convarы с собой, что не очень то круто.

У меня были кейсы:

  1. Плагин с системой запросами/отказами. Есть опция выдачи FreeDay, и чтобы ее выдать, мне приходиться тянуть цвета.
  2. Плагин с автовыдачей фд, тоже самое, приходиться тянуть цвета.

@TiBarification
Copy link
Owner

First of all thank you for contribution. It becomes helpful.
Second: use english when you describe your changes, it makes plugin more relevant for everyone not aligned to know only russian language.

I don't really see why it should be inside a core. If you want to hook whenever color is given from "jwp_freeday" module it can be improved by adding a forward on "jwp_freeday" module instead of modifying core each time. Also inside that module you can override coloring of T team.

@ZakharYA
Copy link
Author

Then yes, it would be great to have a forward to install FreeDay inside the "jwp_freeday" plugin itself.
I need to get the current colors for T now, while not pulling сonvar from "jwp_freeday".

@ZakharYA
Copy link
Author

And by this logic, it makes sense to remove the installation freeday from the core, if we set the colors in a separate plugin, which is not logical.

@TiBarification
Copy link
Owner

I think better would be to completely remove 'freeday' logic from core. Core doesn't need to be dependent on 'freeday' module. Instead modules what are using 'freeday' functions may depend on 'jwp_freeday' itself. It will make Core simplier. Natives can be kept and extended in 'freeday' module.

Maybe it will be a little harsh, but in a perspective it is a good decision.
Same thing might be done on 'rebel' mechanism and other functions not related to core itself.

@ZakharYA
Copy link
Author

It would be great

@TiBarification
Copy link
Owner

These natives might be moved to their corresponding modules.
https://github.com/TiBarification/Jail-Warden-Pro/blob/master/addons/sourcemod/scripting/include/jwp.inc#L235-L307

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants