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

EventCallback Lib a more modern and easy way to configure #3834

Merged
merged 12 commits into from
Feb 18, 2022
Merged

EventCallback Lib a more modern and easy way to configure #3834

merged 12 commits into from
Feb 18, 2022

Conversation

MillhioreBT
Copy link
Contributor

@MillhioreBT MillhioreBT commented Dec 5, 2021

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

I changed the metatable a bit, now we can access it in a more beautiful way.
These changes allow a special configuration for network events, more information about this special in PR #3838

Issues addressed: Nothing!

some small but worth mentioning improvements on line 113, previously 142, checking a boolean is faster than iterating a table to check if a value exists or not
@MillhioreBT
Copy link
Contributor Author

I plan to change something soon, so the opinions can wait please, if you have suggestion, here I am

EPuncker
EPuncker previously approved these changes Dec 7, 2021
@MillhioreBT MillhioreBT mentioned this pull request Dec 7, 2021
3 tasks
@DSpeichert
Copy link
Member

I plan to change something soon, so the opinions can wait please, if you have suggestion, here I am

So is it ready or do you still intend to modify it?

@MillhioreBT
Copy link
Contributor Author

I will remove the byte support, and I will be done, today before I go to sleep I will

EPuncker
EPuncker previously approved these changes Dec 15, 2021
@Znote
Copy link
Member

Znote commented Dec 20, 2021

Could you post some updated code samples?
Does this break compatibility with prior versions?

Merge in blocked due to file conflict.

EPuncker
EPuncker previously approved these changes Dec 20, 2021
@MillhioreBT
Copy link
Contributor Author

Could you post some updated code samples? Does this break compatibility with prior versions?

Merge in blocked due to file conflict.

he same, I have simply modified the library to make it more readable, and in theory a little more efficient

EPuncker
EPuncker previously approved these changes Jan 4, 2022
DSpeichert
DSpeichert previously approved these changes Feb 6, 2022
EvilHero90
EvilHero90 previously approved these changes Feb 15, 2022
@EPuncker
Copy link
Contributor

EPuncker commented Feb 15, 2022

there is a conflict, needs rebase

@EPuncker EPuncker dismissed stale reviews from EvilHero90, DSpeichert, and themself via c117f7c February 18, 2022 20:55
@EPuncker EPuncker merged commit 27e2b46 into otland:master Feb 18, 2022
Codinablack pushed a commit to Codinablack/forgottenserver that referenced this pull request Apr 5, 2022
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