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

[Fix] Binder net packet reader io #1464

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

Neloreck
Copy link
Contributor

Changes

  • Fixing case when writing size_t to net packet, but reading u32, creates issues with custom binders for weapon objects and try to read data

@Xottab-DUTY Xottab-DUTY added Bug The issue in the run-time. x64 64-bit related. Breaking change This breaks saves compatibility, or changes binary formats, etc. labels Oct 16, 2023
@Xottab-DUTY
Copy link
Member

Xottab-DUTY commented Oct 16, 2023

Ohhh, this will introduce incompatibility between x64 and x86 game saves (they won't be identical after this change)
Oh, they aren't compatible already. This needs to be fixed.

@Xottab-DUTY
Copy link
Member

Can't look at the engine code right now to find the place where we actually write size_t to a net packet, so I just briefly say that we should never use size_t in net packets and binary formats, so we need to change the code to write u32 as it was before.
(or u64 if it's reaaally necessary)

@Neloreck
Copy link
Contributor Author

I would stick with u32. Then save method should be modified instead

@Neloreck
Copy link
Contributor Author

We write size of magazine here

    save_data(m_magazine2.size(), output_packet);

And get

    _NODISCARD _CONSTEXPR20 size_type size() const noexcept {
        auto& _My_data = _Mypair._Myval2;
        return static_cast<size_type>(_My_data._Mylast - _My_data._Myfirst);
    }

Should I just do static cast to u32 to make it work?
I believe it is very unlikely to get overflow here + it will match original 32bit engine

@Xottab-DUTY
Copy link
Member

@Neloreck I agree!

@Xottab-DUTY
Copy link
Member

Now we need to analyze all changes we have and merge everything that breaks saves compatibility within one time period (e.g. a day?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking change This breaks saves compatibility, or changes binary formats, etc. Bug The issue in the run-time. ready to merge x64 64-bit related.
Projects
Status: Postponed
Development

Successfully merging this pull request may close these issues.

2 participants