Skip to content

Minor changes and code refactor #942

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Minor changes and code refactor #942

wants to merge 1 commit into from

Conversation

GermanAizek
Copy link

@s1lentq hello, review PR changes and feedback me.

@fred0r
Copy link

fred0r commented Mar 11, 2023

been using this pr for some weeks now on linux with regamedll_cs and the hitboxes 'feel' much more 'accurate/natural' than before.
the downside is that the hltv often gets disconnected with Dropped from x.x.x.x:27015 (Reliable channel overflowed).
in my setup the server records every map played.

@justgo97
Copy link

been using this pr for some weeks now on linux with regamedll_cs and the hitboxes 'feel' much more 'accurate/natural' than before. the downside is that the hltv often gets disconnected with Dropped from x.x.x.x:27015 (Reliable channel overflowed). in my setup the server records every map played.

The PR doesn't include any changes to how hitboxes function.

@GermanAizek
Copy link
Author

@fred0r @justgo97 I had already forgotten about this PR, it turns out that someone else answered me. HLTV breakdown is possible due to delete operator or additional checks, I can divide this PR into two parts, where refactoring and where fixes.

@GermanAizek
Copy link
Author

@justgo97 rehlds project is still active, are you doing it? In September, I created a PR, but no one answered it.

@justgo97
Copy link

@justgo97 rehlds project is still active, are you doing it? In September, I created a PR, but no one answered it.

I'm not the project manager, I'm a normal user like you, I guess you can try to divide the PR to address one issue at a time, It will be easier to review and test the changes, no guarantee you will get an answer from the maintainers though.

@s1lentq s1lentq force-pushed the master branch 2 times, most recently from 43df0ba to de3679f Compare September 20, 2023 14:00
@GermanAizek
Copy link
Author

@s1lentq I accidentally made a force push.
I hope you have pick up necessary fixes?
Added new commit with fix minor memleak 1fa34c2

@stamepicmorg stamepicmorg requested a review from s1lentq April 10, 2025 21:38
@stamepicmorg stamepicmorg added Priority: 🕒 low Low priority tasks that can be postponed for the future. Status: 🖐 available Task currently opened. Type: 🛠️ refactor Refactoring code without adding new functionality. Difficulty: 🟡 medium Task of medium difficulty requiring moderate effort. Engine: ⚙️ Independent Case do not refer to any Engine. OS: 💻 Independent Case do not refer to any OS. labels Apr 10, 2025
s1lentq
s1lentq reviewed Apr 10, 2025
View reviewed changes
rehlds/HLTV/Core/src/BSPModel.h
public:
BSPModel() {}
virtual ~BSPModel() {}
virtual ~BSPModel() { delete m_wadpath; }
Copy link
Member

@s1lentq s1lentq Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

m_wadpath should be released by Free function instead of delete operator, because the pointer was allocated using _strdup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@s1lentq s1lentq s1lentq left review comments

Assignees

@s1lentq s1lentq

Labels

Difficulty: 🟡 medium Task of medium difficulty requiring moderate effort. Engine: ⚙️ Independent Case do not refer to any Engine. OS: 💻 Independent Case do not refer to any OS. Priority: 🕒 low Low priority tasks that can be postponed for the future. Status: 🖐 available Task currently opened. Type: 🛠️ refactor Refactoring code without adding new functionality.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@GermanAizek @fred0r @justgo97 @s1lentq @stamepicmorg