Begin to break into modules #625

TheByKotik · 2019-10-15T04:08:17Z

Motivation and Context

Begin to break into modules for preventing duplication of code.
Also resolve #464.

How Has This Been Tested?

I tested it in different situations.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.

up

game/addons/sourcemod/scripting/sbpp/core.sp

rumblefrog · 2019-10-15T04:15:03Z

game/addons/sourcemod/scripting/sbpp/sql.sp

+enum { /* States */
+ SBPP_SQL_State_None = 0,
+ SBPP_SQL_State_Connecting = 1 << 0,
+ SBPP_SQL_State_Wait = 1 << 1,
+ SBPP_SQL_State_Connected = 1 << 2, }


Indentation seems to be all over the place with this enum

rumblefrog · 2019-10-15T04:17:50Z

game/addons/sourcemod/scripting/sbpp/sql.sp

+ SBPP_SQL_State_Wait = 1 << 1,
+ SBPP_SQL_State_Connected = 1 << 2, }
+
+forward Action _SBPP_SQL_Find (Database &db, int &iState);


Nit, indentation.

rumblefrog · 2019-10-15T04:27:16Z

game/addons/sourcemod/scripting/sbpp/sql.sp

+stock Database g_dbSQL;
+stock static int s_iState;
+stock static bool s_bIgnoreForward;
+stock static ConnectCallback s_fnCallback; /* For now, Database.Connect() can't have function as pass data. */
+stock static char s_szPrevError[PLATFORM_MAX_PATH];
+stock static Handle s_hSBPP_SQL_Find, s_hSBPP_SQL_Close, s_hSBPP_SQL_Release;


What's the case here for defining symbol usage as stock? Also, this seems like a UB.

If I understand your question correctly, stock is not include if never used.

But if they include that file, will it ever be not used?

If someone forgets to call SBPP_SQL_Init().

TheByKotik · 2019-10-15T04:27:41Z

I'm think build failed because don't set include dir (spcomp flag -i).

game/addons/sourcemod/scripting/sbpp/sql.sp

rumblefrog · 2019-10-15T04:35:09Z

game/addons/sourcemod/scripting/sbpp_sleuth.sp

 #undef REQUIRE_PLUGIN
-#include <sourcebanspp>
+ #include <sourcebanspp>


game/addons/sourcemod/scripting/sbpp_sleuth.sp

This reverts commit b8afb5e.

TheByKotik · 2019-10-15T06:02:58Z

Build fixed but It's not quite include. It's something like a module.

game/addons/sourcemod/scripting/sbpp/sql.sp

CrazyHackGUT · 2019-10-15T14:25:18Z

One connection for all plugins can be implemented too easy, if plugins will be connect to DB in single-thread.

https://sm.alliedmods.net/new-api/dbi/SQL_Connect
persistent argument.

TheByKotik · 2019-10-15T14:30:20Z

One connection for all plugins can be implemented too easy, if plugins will be connect to DB in single-thread.

https://sm.alliedmods.net/new-api/dbi/SQL_Connect
persistent argument.

If i remember correctly, this function called from main game thread and should have caused a lag.

CrazyHackGUT · 2019-10-15T18:03:17Z

When you starts plugin - what's the difference? Few people launch plugins when people play. These peoples should understand any risks related with starting plugin in the middle of the game.

Yes, you remember correctly. This function really causes the lag.

TheByKotik · 2019-10-16T04:23:50Z

When you starts plugin - what's the difference? Few people launch plugins when people play. These peoples should understand any risks related with starting plugin in the middle of the game.

Yes, you remember correctly. This function really causes the lag.

You are certainly right, loading plugins often causes lags.

However, connection to database can occur at different times, including during important game events, if, for example, something happened with db / db server.

Just as I mentioned above, this is something like a module that is designed to stop duplication of code from plugin to plugin.

This reverts commit 05df755.

sneak-it · 2020-01-17T21:52:17Z

game/addons/sourcemod/scripting/sbpp/sql.sp

+
+stock void SBPP_SQL_Init (const ConnectCallback fnCallback)
+{
+ LoadTranslations( "sbpp.sql.phrases.txt" );


Seems unnecessary to add a separate translation file for one phrase, this would do better added into sbpp_main.phrases.txt

Seems unnecessary to add a separate translation file for one phrase, this would do better added into sbpp_main.phrases.txt

I planned that this module will be included not only in sbpp_main. Therefore, it would be nonsensical to add a dependency on sbpp_main translations, it is desirable that each plugin can work absolutely independently.

game/addons/sourcemod/scripting/sbpp/core.sp

Groruk · 2020-03-24T18:46:35Z

game/addons/sourcemod/scripting/sbpp/core.sp

+ BuildPath( Path_SM, szLog, sizeof szLog, "logs/sbpp.log" );
+ BuildPath( Path_SM, szIssues, sizeof szIssues, "logs/sbpp.issues.log" );


Would change the name to sbpp.error.log

Suggested change

BuildPath( Path_SM, szLog, sizeof szLog, "logs/sbpp.log" );

BuildPath( Path_SM, szIssues, sizeof szIssues, "logs/sbpp.issues.log" );

BuildPath( Path_SM, szLog, sizeof szLog, "logs/sbpp.log" );

BuildPath( Path_SM, szIssues, sizeof szIssues, "logs/sbpp.error.log" );

Most of the errors are logged by sourcemod itself, most often there will be exactly problems there like can't connect to or can't find file.
My english is not perfect, but they should be called as issues in this case, right?

error is the more universally adapted name, especially when it comes to logs and log levels. My intention behind that change is to make the naming more inline with SM and the other software stack that is needed to run SBPP (e.g nginx / Apache, PHP, etc) since they all use error logs and not issue logs.

~~Renamed.~~ (Absolute forgot about string)

Renamed. Sorry for that.

Replaced static function variable to DataPack to store callbacks. Renamed `Issues` log to `Error`. Fixed `sbpp` dir permisson. Fixed tabulation. Changed ConnectCallback signature. Clean-up static forwards params.

TheByKotik added 5 commits October 10, 2019 10:40

Merge pull request #1 from sbpp/v1.x

9a11fa3

up

Single SQL handle. Init.

4f6185b

Remove debug string.

b886d9a

fixes and code style cleanup

567c2c9

Update sql.sp

5ea54da

rumblefrog reviewed Oct 15, 2019

View reviewed changes

game/addons/sourcemod/scripting/sbpp/core.sp Outdated Show resolved Hide resolved

Cleanup. Again.

26bb09f

rumblefrog added s: review needed s: test needed semver: major t: database/schema t: plugin template: used Issue is written in the issue template type: enhancement labels Oct 15, 2019

rumblefrog reviewed Oct 15, 2019

View reviewed changes

Trying fix test build

b8afb5e

rumblefrog reviewed Oct 15, 2019

View reviewed changes

rumblefrog requested a review from Groruk October 15, 2019 04:30

rumblefrog reviewed Oct 15, 2019

View reviewed changes

game/addons/sourcemod/scripting/sbpp/sql.sp Outdated Show resolved Hide resolved

rumblefrog reviewed Oct 15, 2019

View reviewed changes

game/addons/sourcemod/scripting/sbpp_sleuth.sp

#undef REQUIRE_PLUGIN

#include <sourcebanspp>

#include <sourcebanspp>

Copy link

Member

rumblefrog Oct 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit

rumblefrog reviewed Oct 15, 2019

View reviewed changes

game/addons/sourcemod/scripting/sbpp_sleuth.sp Show resolved Hide resolved

TheByKotik added 4 commits October 15, 2019 09:52

My fault.

48a8d29

New charset

096c528

Revert "Trying fix test build"

e0f0669

This reverts commit b8afb5e.

Moved it to include dir for now. Trying fix builder.

05df755

rumblefrog reviewed Oct 15, 2019

View reviewed changes

game/addons/sourcemod/scripting/sbpp/sql.sp Outdated Show resolved Hide resolved

Fix by project style.

10b7cce

Optimization "freeing" prev error string.

379878d

TheByKotik mentioned this pull request Oct 19, 2019

ci: Update builders. #627

Merged

7 tasks

Revert "Moved it to include dir for now. Trying fix builder."

4647e67

This reverts commit 05df755.

TheByKotik mentioned this pull request Oct 30, 2019

Fix types errors on 1.11. #628

Closed

7 tasks

sneak-it suggested changes Jan 17, 2020

View reviewed changes

Groruk requested changes Mar 24, 2020

View reviewed changes

v2

02c2fc2

TheByKotik changed the title ~~Use one database connection~~ Begin to break into modules Mar 27, 2020

TheByKotik added 3 commits April 20, 2020 17:14

v3

1cb2fa4

Replaced static function variable to DataPack to store callbacks. Renamed `Issues` log to `Error`. Fixed `sbpp` dir permisson. Fixed tabulation. Changed ConnectCallback signature. Clean-up static forwards params.

Fix build with older SM versions

35c5d38

Renamed Issues log to Errors.

0adf6e7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Begin to break into modules #625

Begin to break into modules #625

TheByKotik commented Oct 15, 2019 •

edited

rumblefrog Oct 15, 2019

rumblefrog Oct 15, 2019

rumblefrog Oct 15, 2019

TheByKotik Oct 15, 2019

rumblefrog Oct 15, 2019

TheByKotik Oct 15, 2019 •

edited

TheByKotik commented Oct 15, 2019

rumblefrog Oct 15, 2019

TheByKotik commented Oct 15, 2019

CrazyHackGUT commented Oct 15, 2019

TheByKotik commented Oct 15, 2019

CrazyHackGUT commented Oct 15, 2019

TheByKotik commented Oct 16, 2019

sneak-it Jan 17, 2020

TheByKotik Jan 18, 2020 •

edited

Groruk Mar 24, 2020

TheByKotik Mar 27, 2020

Groruk Mar 27, 2020

TheByKotik Apr 20, 2020 •

edited

TheByKotik May 12, 2020

		BuildPath( Path_SM, szLog, sizeof szLog, "logs/sbpp.log" );
		BuildPath( Path_SM, szIssues, sizeof szIssues, "logs/sbpp.issues.log" );

Begin to break into modules #625

Are you sure you want to change the base?

Begin to break into modules #625

Conversation

TheByKotik commented Oct 15, 2019 • edited

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheByKotik Oct 15, 2019 • edited

Choose a reason for hiding this comment

TheByKotik commented Oct 15, 2019

Choose a reason for hiding this comment

TheByKotik commented Oct 15, 2019

CrazyHackGUT commented Oct 15, 2019

TheByKotik commented Oct 15, 2019

CrazyHackGUT commented Oct 15, 2019

TheByKotik commented Oct 16, 2019

Choose a reason for hiding this comment

TheByKotik Jan 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheByKotik Apr 20, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheByKotik commented Oct 15, 2019 •

edited

TheByKotik Oct 15, 2019 •

edited

TheByKotik Jan 18, 2020 •

edited

TheByKotik Apr 20, 2020 •

edited