Jump to conversation
Unresolved conversations (51)
@SmallJoker SmallJoker Aug 1, 2018
Probably for debug purposes. Should be removed because another backend might come soon.
Outdated
src/serverenvironment.cpp
bendeutsch
@SmallJoker SmallJoker Aug 1, 2018
Mixed indents again EDIT: Also above. Please disable clang format for these lines when they insist on this notation.
Outdated
src/serverenvironment.cpp
bendeutsch
@SmallJoker SmallJoker Jul 31, 2018
Should be at least a warning
Outdated
src/serverenvironment.cpp
@nerzhul nerzhul Jul 29, 2018
you can remove the else and just do a return 0
Outdated
src/script/lua_api/l_auth.cpp
bendeutsch
@nerzhul nerzhul Jul 29, 2018
no need to order names, you can use std::unordered_map
Outdated
src/database/database-files.h
bendeutsch
@nerzhul nerzhul Jul 29, 2018
you can remove the temp variable here
Outdated
src/database/database-files.cpp
bendeutsch
@nerzhul nerzhul Jun 28, 2018
same here
Outdated
src/unittest/test_authdatabase.cpp
bendeutsch
@nerzhul nerzhul Jun 28, 2018
if (x) delete x: conditional is not needed
Outdated
src/unittest/test_authdatabase.cpp
bendeutsch
@sfan5 sfan5 Jun 19, 2018
should be an `std::ostringstream`
Outdated
src/util/string.h
bendeutsch
@Xaleth Xaleth Jun 18, 2018
You want the whole thing to be from a constructor that gets called multiple times when an object is made? Try something sane and have `AuthDatabaseFiles` as a global object and call `readAuthFile()` directly. For example: ( Just an example. don't use this code, it's broken ) ``` extern std::unique_ptr<AuthDatabaseFiles> sqlauth; // Declare the global object. Should be in a header that does global stuff. `std::unique_ptr` to prevent memory leaks. std::unique_ptr<AuthDatabaseFiles> sqlauth = nullptr; // In a .cpp file for the header mentioned above. // In this file, you can just have something call `readAuthFile()` by doing... sqlauth = std::unique_ptr<AuthDatabaseFiles>( new AuthDatabaseFiles() ); sqlauth->readAuthFile(); ``` Boom.
src/database/database-files.cpp
bendeutsch
@SmallJoker SmallJoker May 31, 2018
Mixed indents. Please only use tabs
Outdated
src/serverenvironment.cpp
bendeutsch
@SmallJoker SmallJoker May 31, 2018
These functions need to be documented.
src/script/lua_api/l_auth.cpp
SmallJoker bendeutsch
@SmallJoker SmallJoker May 31, 2018
Also add to https://github.com/minetest/minetest/blob/master/build/android/jni/Android.mk EDIT: Same for the unittest file.
src/script/lua_api/CMakeLists.txt
sfan5 SmallJoker
bendeutsch
@sfan5 sfan5 May 29, 2018
brace removal
Outdated
src/serverenvironment.cpp
bendeutsch
@sfan5 sfan5 May 29, 2018
`const auto`
src/serverenvironment.cpp
bendeutsch
@sfan5 sfan5 May 29, 2018
does this even work without a `&`?
src/serverenvironment.cpp
bendeutsch
@sfan5 sfan5 May 29, 2018
can be `auto`
src/script/lua_api/l_auth.cpp
bendeutsch
@sfan5 sfan5 May 29, 2018
this is just an implementation detail of the sqlite db, but I guess it can't be avoided to have it in here.
src/database/database.h
bendeutsch
@sfan5 sfan5 May 29, 2018
is sqlite's (u)int big enough to hold timestamps beyond 2038?
Outdated
src/database/database-sqlite3.cpp
nerzhul raymoo
bendeutsch
@sfan5 sfan5 May 29, 2018
wouldn't `REPLACE INTO` have the same effect?
src/database/database-sqlite3.cpp
nerzhul
@sfan5 sfan5 May 29, 2018
same
src/database/database-files.h
@sfan5 sfan5 May 29, 2018
this can be a const method
src/database/database-files.h
sfan5 bendeutsch
@nerzhul nerzhul May 29, 2018
missing space before parenthesis
Outdated
src/database/database-sqlite3.cpp
bendeutsch
@nerzhul nerzhul May 29, 2018
const auto
Outdated
src/database/database-files.cpp
bendeutsch
@nerzhul nerzhul May 27, 2018
nice :)
src/database/database-sqlite3.cpp
@nerzhul nerzhul May 27, 2018
don't duplicate this code part, use a private function to perform the loop accross the two write functions
Outdated
src/database/database-sqlite3.cpp
bendeutsch
@nerzhul nerzhul May 27, 2018
exactly same problems there.
Outdated
src/database/database-sqlite3.cpp
@nerzhul nerzhul May 27, 2018
code style
Outdated
src/database/database-sqlite3.cpp
bendeutsch
@nerzhul nerzhul May 27, 2018
you must use a begin and commit function to ensure data integrity there in a single write transaction
Outdated
src/database/database-sqlite3.cpp
bendeutsch
@nerzhul nerzhul May 27, 2018
std::time_t
Outdated
src/database/database-files.cpp
bendeutsch
@nerzhul nerzhul May 27, 2018
name & pass should be const
Outdated
src/database/database-files.cpp
bendeutsch
@SmallJoker SmallJoker May 8, 2018
Just to have it said: Last login exists since almost four years but our auth mechanism didn't add dummy values if they didn't exist. However, I think we can consider this as a feature to clean up really old, unused accounts.
Outdated
src/database/database-files.cpp
bendeutsch nerzhul
SmallJoker
@SmallJoker SmallJoker May 8, 2018
Superfluous return EDIT: Also found in functions in the file auth database code.
Outdated
src/database/database-sqlite3.cpp
bendeutsch
@SmallJoker SmallJoker May 8, 2018
Superfluous check and return.
Outdated
src/script/lua_api/l_server.cpp
bendeutsch
@SmallJoker SmallJoker May 8, 2018
Nothing is returned here.
Outdated
src/script/lua_api/l_server.cpp
bendeutsch
@nerzhul nerzhul May 8, 2018
no need for a temp object here, affect it directly
Outdated
src/database/database-files.cpp
bendeutsch
@nerzhul nerzhul May 8, 2018
for (const auto &p: authEntry.privileges)
Outdated
src/database/database-files.cpp
@nerzhul nerzhul May 8, 2018
can you store AuthDatabase pointers in a unique_ptr please ? There are some possible leaks here when exceptions are triggered
Outdated
src/serverenvironment.cpp
bendeutsch nerzhul
@nerzhul nerzhul May 8, 2018
const std::string &
Outdated
src/serverenvironment.cpp
bendeutsch
@nerzhul nerzhul May 8, 2018
Can you make auth be a specific Lua object instead of a static function API ? It can be better to have an auth object to manipulate
Outdated
src/script/lua_api/l_server.cpp
nerzhul bendeutsch
@nerzhul nerzhul May 8, 2018
return 0 should be sufficient here
Outdated
src/script/lua_api/l_server.cpp
bendeutsch
@nerzhul nerzhul May 8, 2018
with the std::next ? for (auto &p : authEntry.privileges) is sufficient
Outdated
src/database/database-sqlite3.cpp
bendeutsch nerzhul
@nerzhul nerzhul May 8, 2018
please add an UNIQUE index on the name field as it's unique, this will prvent the ugly limit 1 suggestion
Outdated
src/database/database-sqlite3.cpp
bendeutsch nerzhul
raymoo
@SmallJoker SmallJoker May 7, 2018
Superfluous. `return 0` does the same, also bracket removal: ```C++ if (!success) return 0; ```
Outdated
src/script/lua_api/l_server.cpp
bendeutsch
@SmallJoker SmallJoker May 7, 2018
This is very inconsistent. On failure you're pushing nil to Lua, but not on success. I recommend to only return the pointers and push nil in the parent function.
Outdated
src/script/lua_api/l_server.cpp
bendeutsch
@SmallJoker SmallJoker May 7, 2018
SELECT and UPDATE: add `LIMIT 1`. It doesn't have to check the entire database. Only update: please use `SET` in capitals (trivial)
Outdated
src/database/database-sqlite3.cpp
bendeutsch nerzhul
@SmallJoker SmallJoker May 7, 2018
Are you getting this auth again only to get the new `id`?
Outdated
src/database/database-sqlite3.cpp
bendeutsch
@SmallJoker SmallJoker May 7, 2018
Wrong formatting here and two lines above too.
Outdated
src/database/database-sqlite3.cpp
@SmallJoker SmallJoker May 7, 2018
Wrong formatting. Ignore Lint if they requested this.
Outdated
src/database/database-sqlite3.cpp
nerzhul SmallJoker
bendeutsch
@Dumbeldor Dumbeldor May 4, 2018
const
Outdated
src/database/database-sqlite3.cpp
bendeutsch
@Dumbeldor Dumbeldor May 4, 2018
const std::string &name
Outdated
src/database/database-files.cpp
bendeutsch
Resolved conversations (0)