Skip to content
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.

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

changing the existing weapon instead of creating a new one #197

Merged
merged 9 commits into from
Mar 27, 2020
Merged

changing the existing weapon instead of creating a new one #197

merged 9 commits into from
Mar 27, 2020

Conversation

BestAwperEver
Copy link
Contributor

@BestAwperEver BestAwperEver commented Mar 27, 2020

Hotfix for the recent internal changes.

Without it we have many players with unknown weapons. Before the internal changes we had an actual array with fixed memory layout so every entity ID had a fixed pointer to an equipment, which doesn't hold now, so when we first have a player binded (with unknown weapons at the moment) and then we bind their weapons, the pointers we have in player's rawWeapons still point to the unknown weapons.

@codecov
Copy link

codecov bot commented Mar 27, 2020

Codecov Report

Merging #197 into master will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #197   +/-   ##
=======================================
  Coverage   92.50%   92.50%           
=======================================
  Files          28       28           
  Lines        2655     2655           
=======================================
  Hits         2456     2456           
  Misses        148      148           
  Partials       51       51           
Impacted Files Coverage Δ
datatables.go 98.97% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3634f34...7014971. Read the comment docs.

datatables.go Outdated Show resolved Hide resolved
@BestAwperEver BestAwperEver requested a review from markus-wa March 27, 2020 17:25
datatables.go Outdated Show resolved Hide resolved
datatables.go Outdated Show resolved Hide resolved
@BestAwperEver
Copy link
Contributor Author

Why does Travis fail, exactly? Locally I ran regression tests on both ParserWarn and unassert variants and it seems to be OK, and also Travis passed here with ParserWarn as well

@markus-wa
Copy link
Owner

travis failed on an unassert condition. to test these you have to run the tests with -tags unassert_panic (or run test scripts from ./bin/ which should do it automatically).

I did some debugging and took the liberty of pushing a fix to your branch: 7c21a15

fingers crossed this works.

@markus-wa
Copy link
Owner

markus-wa commented Mar 27, 2020

I removed the comment Something is clearly wrong here as it seems to be a common case.

I accidentially also removed the other one, oops. will re-add it in a sec

@markus-wa markus-wa merged commit e0887e1 into markus-wa:master Mar 27, 2020
@markus-wa
Copy link
Owner

markus-wa commented Mar 27, 2020

Merged - many thanks as always @BestAwperEver 🙌

v1.9.1 is out as well: https://github.com/markus-wa/demoinfocs-golang/releases/tag/v1.9.1

@BestAwperEver
Copy link
Contributor Author

No problem, was my mistake in the first place 😃

@markus-wa
Copy link
Owner

was my mistake in the first place

Only because I forced you to replace my ugly array with a map 😅

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

Successfully merging this pull request may close these issues.

2 participants