Move Events methods to namespace by ramon-bernardo · Pull Request #4692 · otland/forgottenserver

ramon-bernardo · 2024-05-19T18:22:38Z

Pull Request Prelude

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

Changes Proposed

Move Events methods to namespace.

ramon-bernardo

I need assistance with these points. What decision should I make?

ramon-bernardo · 2024-05-19T18:25:03Z

src/events.h

-};
+namespace tfs::events {
+
+bool load();


I prefer to change this for each events::x, as it gives us better control over each type. However, this approach will result in repeated code and reading the same file multiple times. I suggest this because I plan to add global events and move events to the namespace.

like

tfs::events::global::startup(); tfs::events::global::shutdown(); tfs::events::global::save();

src/events.cpp

ramon-bernardo · 2024-05-19T18:27:00Z

src/combat.cpp

 	}
-
-	return g_events->eventCreatureOnAreaCombat(caster, tile, aggressive);
+	return tfs::events::creature::onAreaCombat(caster, tile, aggressive);


I'm still unsure whether to use tfs::events or tfs::game::events. What do you think?

tfs::events

src/events.cpp

+
+namespace tfs::events {
+
+bool load()


* doubling the vartrack size Increasing the max-vartrack-size because luascript.cpp has a function which exceeded the limit and thus the build failed

MillhioreBT · 2024-05-21T22:09:06Z

@ranisalt to what extent is it good to use namespace to handle all singletons?
I personally like namespaces, but is it in its main function? Anyway, I can't think of any other idea.

This reverts commit 98c55fa.

* Refactor Docker image workflow with docker actions

* Create cpplinter.lua

* Don't install libiconv outside OSX, install Lua by default

@ramon-bernardo

* Use Guild shared_ptr * Member rank level default to constexpr * Use const when possible * @ramon-bernardo Use const when possible * Use shared_ptr alias * Fix * Fix * Fix * Fix * Fix delete * Fix * No need to reuse the variable * Use const shared ptr * Use const& ptr * Inline rank check

* Split game core into different files * eof

ranisalt · 2024-05-26T19:52:38Z

to what extent is it good to use namespace to handle all singletons?

Singletons are ~~almost~~ always a bad idea. Namespaces are an enhanced private section in a class, since you hide the implementation details and the compiler can assume it's not visible and perform more aggressive optimizations.

I like this post on how singletons are an awful idea in general. Namespaces don't solve it, but they make it less bad.

…et (#4725)

thrown an error when it didn't find a single number included in the message

… by 1 (#4733) Co-authored-by: EvilHero90 <Shadow280690@hotmail.de>

The first leak in #4288 is related to handle leaking, I couldn't really reproduce how mysql_init is called without mysql_close not being called on the line before, but using a unique_ptr with a custom deleter fixes the leak. Using unique_ptr for MYSQL_RES also fixes eventual leaks that happen on queries.

* Compilation problem issue template update I've added - Buildtype - Compiler flags for more information Co-authored-by: Ranieri Althoff <1993083+ranisalt@users.noreply.github.com>

- npc now correctly clears all npc id data when he is removed - farewell and greet messages can be individually set per talk state if not they fall back to default - fixed a bug if the npc disappeared while the player was talking to him, then the npc would not respond to the player again once he was spawned - better warning/error messages which show which npc it occured and which file it originates from - added shop callback function callback(npc, player, handler, items, afterDiscount) order: - discount (if there is any) -> callback - added NpcsHandler:player() this is basicly the equivalent to requirements but instead of requireing you can add/set stuff to the player (modules.lua) - implemented modules they're load after callbacks order: requirements (if returns true) -> callback (if returns true) -> modules - moved teleport/addItems/setStorageValue to modules - added new requirements - changed the requirements functions - I've included the error message directly inside the functions - removed :failureRespond & :getFailureRespond as they're redundant with requirement re work - changed modules outfit function it requires to insert 2 different outfits now because of 2 different player sexes ex: :outfit(id1, id2, addon) - a few linter/annotation fixes - if the player changed the talk state but still had the shop open and then tried to purchase something it thrown errors, this is now fixed

Release of The Forgotten Server 1.6

The start of `The Forgotten Server 1.7` Development cycle

* Fix some warnings and improve type annotations * Fix `item:transform` type annotation * Remove `dump` function from compat.lua * Fix `configKeys` type annotations * Refactor all enums/constants of `cpplinter.lua` and fix ident style to tabs Some were missing and others were wrong, I got them all directly from the source. * Refactor the PlayerFlag constants in cpplinter.lua to use bitwise shift operators instead of exponentiation

* Moved everything to NpcScriptInterface There was an issue with the Revisioned Npc System. Because it was included in 2 different lua environments it would not correctly work on any of the cmake builds only the msvc builds worked fine (they somehow have shared lua envs) Now the npcs should correctly work as it's expected

Moved all std::vector out of the pointer class itself and put them into their singleton classes (Actions/MoveEvents) They're now stored in a std::map map by their pointer as a std::vector and once it is registered it clears the std::map with the pointer index.

- checking first if the pointer has a map (is valid) before trying to get it's content

this sets NULL default for session tokens, just for safety

Add monster icons correctly when creature appears on the screen

- Added `onSight` event, this has to be enabled with `NpcType:sight(x, y)` unless that is not set the event wont fire. This event triggers whenever a creature (monsters/npcs/players) steps into the sight range, will only re trigger once you walk out of idle range and back in again. - fixed event callbacks, they're now moved into `NpcCallbacks` they can be set like: `NpcType:callbacks().onSay/onSight/etc.` - fixed a crash which could occur if you had a npc disabled then enabled him and /reload npcs, this is now fixed - fixed a crash that if a xml npc is invalid on server startup that it throws an error instead of crashing - made all the functions in the lib overloadable, only the table data is protected, you can now change functions and they will be correctly changed on /reload npcs - changed everything related to focus from seconds into ms - changed focus so it works for all creatures not only players - changed NpcTalkQueue it now works for creatures not only for players - changed the onCreatureSay event, it now triggers for all creatures not only for players - selfSay has a new parameter now called talkType which is optional and falls back to TALKTYPE_SAY - added `onSpeechBubble` event, with this we can set the speech bubble for each player individual. example: Npc has a quest for the player he only shows for this player the quest speech bubble, once the quest is finished the npc wont have a quest speech bubble anymore --------- Co-authored-by: Evil Puncker <EPuncker@users.noreply.github.com> Co-authored-by: KrecikOnDexin <krecikondexin@gmail.com>

* Remove Account class and account.h file * Update theforgottenserver.vcxproj

…forgottenserver into events-namespace

Move Events methods to namespace

4706229

ramon-bernardo commented May 19, 2024

View reviewed changes

Rename classname

b90be76

github-advanced-security bot found potential problems May 19, 2024

View reviewed changes

src/events.cpp Fixed Show fixed Hide fixed

ranisalt reviewed May 19, 2024

View reviewed changes

src/events.cpp Outdated Show resolved Hide resolved

ranisalt requested review from EvilHero90 and MillhioreBT May 19, 2024 21:29

Fix

197d007

github-advanced-security bot found potential problems May 20, 2024

View reviewed changes

src/events.cpp

namespace tfs::events {

bool load()

Check warning

Code scanning / CodeQL

Poorly documented large function

Poorly documented function: fewer than 2% comments for a function of 136 lines.

ranisalt and others added 2 commits May 21, 2024 16:02

Run unit tests with vcpkg (#4695)

349cd8c

Docker Build failing (workflow) (#4696)

98c55fa

* doubling the vartrack size Increasing the max-vartrack-size because luascript.cpp has a function which exceeded the limit and thus the build failed

ranisalt and others added 10 commits May 22, 2024 01:42

Revert "Docker Build failing (workflow) (#4696)" (#4700)

ea0083a

This reverts commit 98c55fa.

Fix Dockerfile copying executable (#4699)

ac4f939

Refactor Docker image workflow with docker actions (#4701)

dd63567

* Refactor Docker image workflow with docker actions

Fix docker registry in workflow (#4702)

bc62acb

Add package write permission to docker-image job (#4703)

52896ab

Annotations & Linter including luascript.cpp functions (#4672)

ea59250

* Create cpplinter.lua

Use repository name when uploading Docker image (#4704)

a271374

Simplify logic to select Lua interpreter (#4576)

4789ce0

* Don't install libiconv outside OSX, install Lua by default

Fix bank NPC overflow protection being bypassed (#4693)

33e9698

ramon-bernardo requested a review from ranisalt May 26, 2024 13:11

ramon-bernardo marked this pull request as ready for review May 26, 2024 13:12

Split game core into different files (#4705)

5601a62

* Split game core into different files * eof

ranisalt and others added 4 commits May 26, 2024 23:15

Remove Ubuntu build workflow (#4707)

ce0cc0c

Fix LuaJIT flag in CMake configuration (#4711)

b85757f

Move static LuaScriptInterface methods to namespace (#4675)

55c0462

actions: Add CodeQL back (#4712)

4f90414

ranisalt and others added 28 commits June 2, 2024 23:41

Set correct minimum dependency versions (#4720)

855cd3b

Fix CodeQL to run only once per workflow (#4721)

dc91b3d

Fix GitHub workflow syntax error (#4723)

75cefe8

[bugfix] default duration on decaying items with no min or maxvalue s…

262652b

…et (#4725)

[bugfix] fixes bug when player asks banker to "withdraw" "x" (#4732)

e9b6383

thrown an error when it didn't find a single number included in the message

Fix use-after-free in creature conditions (#4726)

3d7c2fe

[bugfix] make it so that 1% resistance correctly reduces damage taken…

e3cae60

… by 1 (#4733) Co-authored-by: EvilHero90 <Shadow280690@hotmail.de>

Compilation Error issue template update (#4736)

553c79a

* Compilation problem issue template update I've added - Buildtype - Compiler flags for more information Co-authored-by: Ranieri Althoff <1993083+ranisalt@users.noreply.github.com>

Fix default database version (#4737)

d8e58ee

The Forgotten Server 1.6 (#4718)

0986419

Release of The Forgotten Server 1.6

Fix paths in release action (#4738)

28a58ec

The Forgotten Server 1.7 Development (#4719)

c677f03

The start of `The Forgotten Server 1.7` Development cycle

Added missing default dependencies for using vcpkg manifest (#4735)

7011488

[Revscriptsys] fix crash introduced in #4742 (#4750)

d59a5cb

- checking first if the pointer has a map (is valid) before trying to get it's content

fix session table in schema (#4745)

404f369

this sets NULL default for session tokens, just for safety

Fix creature icons (#4746)

00f4400

Add monster icons correctly when creature appears on the screen

Remove g_bans global (#4757)

48352f6

Remove Account class and account.h file (#4730)

a2b24a4

* Remove Account class and account.h file * Update theforgottenserver.vcxproj

Fix NPC baseSpeed & build issue (AccountType_t) (#4763)

98ff8a8

Move Events methods to namespace

d11bf08

Fix

fc8d0c6

Merge branch 'events-namespace' of https://github.com/ramon-bernardo/…

6d7a18c

…forgottenserver into events-namespace

ranisalt mentioned this pull request Aug 26, 2024

Move static IOBan methods to namespace #4777

Merged

3 tasks

ramon-bernardo closed this Sep 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Events methods to namespace#4692

Move Events methods to namespace#4692
ramon-bernardo wants to merge 54 commits intootland:masterfrom
ramon-bernardo:events-namespace

ramon-bernardo commented May 19, 2024

Uh oh!

ramon-bernardo left a comment

Uh oh!

ramon-bernardo May 19, 2024 •

edited

Loading

Uh oh!

Uh oh!

ramon-bernardo May 19, 2024 •

edited

Loading

Uh oh!

ranisalt May 19, 2024

Uh oh!

Uh oh!

Uh oh!

Check warning

MillhioreBT commented May 21, 2024

Uh oh!

ranisalt commented May 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

Conversation

ramon-bernardo commented May 19, 2024

Pull Request Prelude

Changes Proposed

Uh oh!

ramon-bernardo left a comment

Choose a reason for hiding this comment

Uh oh!

ramon-bernardo May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ramon-bernardo May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranisalt May 19, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Check warning

MillhioreBT commented May 21, 2024

Uh oh!

ranisalt commented May 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

ramon-bernardo May 19, 2024 •

edited

Loading

ramon-bernardo May 19, 2024 •

edited

Loading