• There is NO official Otland's Discord server and NO official Otland's server list. The Otland's Staff does not manage any Discord server or server list. Moderators or administrator of any Discord server or server lists have NO connection to the Otland's Staff. Do not get scammed!
  • If you're using Gesior 2012 or MyAAC, please review this thread for information about a serious security vulnerability and a fix.

opentibiabr code 'quality' and community

Gesior.pl

Mega Noob&LOL 2012
Senator
Premium User
Joined
Sep 18, 2007
Messages
2,834
Solutions
92
Reaction score
2,984
Location
Poland
GitHub
gesior
I've made issue describing critical bug in canary.. it was deleted :(

Maybe my issue language wasn't nice, but there were no personal insults, just description of problem.
I was a bit upset, as it's the same bug that I've fixed year ago. Someone added it again after 6 months.
Now I'm BANNED on opentibiabr project on GitHub. I cannot create new Issues or comment :)

I don't see any repost of this issue or PR related to it on opentibiabr GitHub.
Do admins of opentibiabr project try to hide that bug? Do they plan to sell fix for it to opentibiabr server owners?

When I saw otservbr code for first time, I was pretty sure it was made by big-ots-owners to lure newbies and crash their servers, when they get popular.
It's just impossible to do so many so dumb mistakes in code.

For anyone interested. These lines:
are executed in network thread and modify player/map/tile. You cannot do it safely in network thread.
It results in random crash reports from gdb with functions like getSpectators, checkCreatures, clearSpectatorCache and other related to map and movement.
You can remove that code or move it to 'dispatcher thread', as I did with all packet parsing year ago:
 

Beatss

New Member
Joined
Oct 29, 2021
Messages
4
Reaction score
2
GitHub
BeatsDh
Look how cool, here your conversation was already much cooler than the conversation you opened in the issue!!!!
I've made issue describing critical bug in canary.. it was deleted :(

Maybe my issue language wasn't nice, but there were no personal insults, just description of problem.
I was a bit upset, as it's the same bug that I've fixed year ago. Someone added it again after 6 months.
Now I'm BANNED on opentibiabr project on GitHub. I cannot create new Issues or comment :)

I don't see any repost of this issue or PR related to it on opentibiabr GitHub.
Do admins of opentibiabr project try to hide that bug? Do they plan to sell fix for it to opentibiabr server owners?

When I saw otservbr code for first time, I was pretty sure it was made by big-ots-owners to lure newbies and crash their servers, when they get popular.
It's just impossible to do so many so dumb mistakes in code.

For anyone interested. These lines:
are executed in network thread and modify player/map/tile. You cannot do it safely in network thread.
It results in random crash reports from gdb with functions like getSpectators, checkCreatures, clearSpectatorCache and other related to map and movement.
You can remove that code or move it to 'dispatcher thread', as I did with all packet parsing year ago:
PR hasn't been opened yet, but there's a branch with modifications already, available here: Comparing main...fix-online-duplication · opentibiabr/canary (https://github.com/opentibiabr/canary/compare/fix-online-duplication?expand=1)
 
Last edited:

Night Wolf

I don't bite.
Joined
Feb 10, 2008
Messages
543
Solutions
7
Reaction score
836
Location
Spain
You could have:
1. Opened a PR
2. Write a wiki explaining the behavior of network dispatcher thread and the importance of making those calls safe
3. Write a polite issue pointing out what was wrong and support us correcting AND TESTING in the meantime

What you did:
1. Offended the team
2. Implied that they are working for years straight day and night just so they can scam a server with 60 players online (that's what big ots are after all, right?)
3. Complained without supporting, like everyone else in this community seems to do
4. Said if we didn't fixed in a few days you would go to otland and say the project is corrupt
5. Actually went to otland to flame the project once again
:p


Hmm, I really wonder what got your issue deleted and you banned from the github repository... such mistery

Do admins of opentibiabr project try to hide that bug? Do they plan to sell fix for it to opentibiabr server owners?
Yes, naturally the most obvious reason why a bug have re-surfaced in the code is because of this and not because several pieces of code are being rewritten everyday. Everyone here works in full time jobs worlwide, but the gross part that sustain our incredible and expensive lives is actually incoming from selling fixes to open tibia, right? 🤣

When I saw otservbr code for first time, I was pretty sure it was made by big-ots-owners to lure newbies and crash their servers, when they get popular.
It's just impossible to do so many so dumb mistakes in code.
When you saw it the first time, it was still a 1:1 match of TFS hahahahaha at least we agree at something
 

guispiller

New Member
Joined
Nov 29, 2021
Messages
10
Reaction score
4
GitHub
guispiller
Yes, as you can see, you're the one who everyone looks for to buy fixes, no one ever reported this before, so that means you're paid by canary members? KEKW
So, for 6 months you're hiding this info again, for your own purposes? You see how easy it is to make assumptions?

As you can see, Canary is constantly being updated, sadly this error passed by, and no one reported.
The user that reported to you, why did he went directly to you? If he tried to open an issue respectfully, or asked anywhere, every smalltalk here would be avoided.

An Open Source Community has no owners, only contributors and Admins, to take out the toxic users or whoever uses something to get advantage. You know the reason you're banned, mind your manners bro, you're not on the street.

We could thank you for your report, but instead you managed to get banned from the community. We don't need no showman at the community.

There's a branch with some fixes being made and this will be fixed.
If you want to talk like a big boy, there's room for everyone to contribute :)
 

amatria

Well-Known Member
Joined
May 15, 2021
Messages
74
Solutions
6
Reaction score
78
Location
Spain
GitHub
amatria
This is the result of bad contributing practices and a vague review process, in my opinion.
  • No Contributing guidelines.
  • The Pull Request (#378) that introduced the race condition again not only "resolved" a total of 5 issues, but it also added 3 new features on top of those "fixes".
  • Divide and conquer? Narrow the scope of the reviews?
  • The changes do not appear to have been tested, even though the Pull Request template has a separate section dedicated to that matter.
  • Too many quality assurance procedures, plus automatic actions, plus etc. etc., yet the only code smell pointed out by sonar is that you can narrow the data type of an iterator.
  • The Pull Request introduced duplicate code. I understand that sonar can fail, but given that barely a handful of lines of logic were modified it is hard to believe that no reviewer pointed that out.
  • All the code modifying the game's state in protocolgame.cpp is safeguarded by calls to the dispatcher thread. The Pull Requests introduces changes that modify the game's state in protocolgame.cpp unsafely. No reviewer pointed that out.
  • A total of four reviewers approved the changes.
  • Only two reviewers added valuable feedback into the review process.
  • Three reviewers approved the Pull Request on the same date after several days of inactivity.
  • etc. etc.
 
Last edited:

guispiller

New Member
Joined
Nov 29, 2021
Messages
10
Reaction score
4
GitHub
guispiller
This is the result of bad contributing practices and a vague review process, in my opinion.
  • No Contributing guidelines.
  • The Pull Request (#378) that introduced the race condition again not only "resolved" a total of 5 issues, but it also added 3 new features on top of those "fixes".
  • Divide and conquer? Narrow the scope of the reviews?
  • The changes do not appear to have been tested, even though the Pull Request template has a separate section dedicated to that matter.
  • Too many quality assurance procedures, plus automatic actions, plus etc. etc., yet the only code smell pointed out by sonar is that you can narrow the data type of an iterator.
  • The Pull Request introduced duplicate code. I understand that sonar can fail, but given that barely a handful of lines of logic were modified it is hard to believe that no reviewer pointed that out.
  • All the code modifying the game's state in protocolgame.cpp is safeguarded by calls to the dispatcher thread. The Pull Requests introduces changes that modify the game's state in protocolgame.cpp unsafely. No reviewer pointed that out.
  • A total of four reviewers approved the changes.
  • Only two reviewers added valuable feedback into the review process.
  • Three reviewers approved the Pull Request on the same date after several days of inactivity.
  • etc. etc.
Yep, that would be the best world for us, sadly, most of the PRs remains opened for months with no one outside testing, we try to test the most, but sometimes some small issues pass unnoticed.

If you really know what happened about this PR, since it wasn't told which PR it was from, you really could have pointed it out first, it would help a lot, really.

Everyone is welcome if have something to comment, criticize, give ideas that can add up to the project, but everything respectfully of course. If the main goal is not to despise anyone.
 

EduardoDantas

Intermediate OT User
Joined
Sep 25, 2013
Messages
190
Solutions
5
Reaction score
137
Location
Brazil
The community level is degrading... This thread is an example of that. We have as author a person who sells "fix issues" for the project and accuses the maintainers and contributors of putting problems on purpose so that "the maintainers and contributors can sell "corrections", as if that wasn't what the accuser does... And still have the courage to make fun of the person who contributes.

In the comments we have people who don't understand and who never helped or contributed to the project, creating fetched theories of how everyone could be better. And others creating new accounts to call the maintainers and contributors monkeys, and to talk a lot of shit... This forum has seen better days, including moderation, which turns a blind eye to everything that is said about canary/otbr (some reason unknown).

My compliments, this thread is a real waste of time.
 
Last edited:

Gluttony

Member
Joined
Nov 4, 2022
Messages
21
Solutions
1
Reaction score
13
your opinion based on what exactly, sorry?
You have 0 contributions to the project, quite few to the tfs itself. Even less at otland and honestly your github is timid to say the least for someone who's a soft eng with PHD.
Don't you think you have literally no materialist argumentation or understandment of the context and is just throwing off a bunch of "theory of best practices" that have nothing to do with a project which the goal isn't stability nor a company?
Computer SCIENCE has science in its name because you need to follow the epistemology to achieve a conclusion. Didn't they teached you this at your graduation/master/phd?
The fact that you believe you are an enlighted being, who knows the solution and all others that have way more study, experience and participation than you are just shit heads, shows more about you than any conclusion you believe you arrived.
Now, let me explain to you why your argument is wrong: you lack the context;

The project is maintained by 5 people. Contributing guidelines is something that is not necessary even when you have projects with 1000+ active maintainers. Most people that know github (all devs in the world by now) knows how they should contribute.

Debatable, while it's indeed a best practice to separate the changes for a better history (and easy to revert) he actually done that in the commits.
The "issues resolved" as you're mocking were tested and confirmed, but you don't believe in evidence just because a race condition was introduced, right?

To divide and conquer you need to have people to divide the work with, have you seen the state of the repositories? For you to suggest something first assume you don't know the problem.
People do what they can, with limited knowledge and time. This project is far from a company and the goal was never stability (according to many here, if we wanted stability it's just never changing the code)

The only untested change (out of 22 commits) was this Fix checks on player death, player speed breakpoint and misc enhancements by omeranha · Pull Request #378 · opentibiabr/canary (https://github.com/opentibiabr/canary/pull/378/commits/ce15dfba16cea35af651bc52666968f7ac2757ac)
The one that introduced the racing condition back.


you want us to do what, divide our limited time contributing to sonar repository too?


I reviewed everything again and couldn't find the duplicated code, since you spotted it why didn't you left a note?
You see why you're also part of the problem? You come here pointing fingers but never actually did anything to support, and worst: your opinion can't even be respected because it lacks applicability in the real world scenario.


Again, your problem here is that you believe we are a multinational working with 100+ employees and that everyone is a senior developer with deep knowledge in program safety. If you have that background, why don't you spare 2h per day to review all PRs in deep detail?
Or even better: why don't you realize this is a hobby for most of us and that absolutely neither of us have a server to test and in many situations since the users don't actively test/contribute with changes/reviews we rely on applying things in main so we can catch the errors?
Now you need to understand that a project like this isn't being used in PROD scenario. Big servers using our base don't have open source forked versions that are just pulling changes, so in essence our PROD environment is their DEV/PRE-PROD because they will pull, test it and select what will enter and what won't. If they don't do that, the problem is still theirs. Gesior even pointed out that people REACHED HIM to get this fixed (buying this as a service), no one tested our PR, no one contributed or said it was wrong. Gesior sold this correction to people and never actually contributed to the repository, all he did was raise an issue insulting our work.



That just means the code was analyzed statically but not under runtime, through a review you can't catch a race condition and I'm not even going to say you should know that because it's obvious for me that you know that.


we discuss through internal chats because it's quicker, don't you do the same in your company? If not, man, I really understand why they keep importing people from other countries to take jobs in Spain 😂


we discussed internally and approved all at once. I don't even understand why this is listed as a point(?)
Clarified? Any other incredible lessons from Amatria, the wise?
Actually gesior helps many times without any money, and here he tried to fix your broken engine, then you cry cuz he wasn't polite. I bet its you who made this bug.
 

Peonso

Godly Member
Joined
Jan 14, 2008
Messages
1,713
Solutions
30
Reaction score
1,454
Seems like we came full circle, I can't miss this joke, it's just too funny.

There is always an excuse for everything.
Almost every pull request that leads to a significant change, difficulties and excuses are placed. This totally breaks the mood for anyone to continue contributing.
 
Top