Rename mcproto.packets.abc to mcproto.packets.packet

[Martysh12]

No description provided.

[ItsDrike]

Nice, but this is a breaking change, not an internal one, mcproto.packets.abc was absolutely a part of the public API, and people using this library are almost certainly importing something from there.

It might then also be worth it to consider whether we should add deprecations here, if not, a major version bump will be necessary for the next release containing this commit, which I'm not sure I like, as simple bugfix releases might need to be made before 1.0.0 is ready, and this would block all such releases due to an introduction of a breaking change.

Without deprecations, this PR is blocked until 1.0.0 is closer to being ready. Alternatively, this could get merged into a new temporary development branch for the 1.0.0 release, keeping the main branch without any such breaking changes, making it easy to release bugfixes if necessary, and keep the development of new things inside of this development branch.

[ItsDrike]

Blocked until 1.0.0 release comes closer, due to an introduction of a breaking change.

[ItsDrike]

So, what's the reason for this rename? abc here suggest this file is holding abstract base classes, it's fairly common to see files named like this in many projects, why is packet better here?

Perhaps you assumed abc was a placeholder name which didn't mean anything, and was only used for the lack of better name? If so, that actually wasn't the case here. Any reason why you think the packet name works better here?

[Martysh12]

We've talked about this in Discord, and I'm pretty sure you're the one who suggested it:

Also, sorry for being inactive in this project for such a long time. I've been working on other things.

[ItsDrike]

We've talked about this in Discord, and I'm pretty sure you're the one who suggested it:

Ah, right, yeah I completely forgot about that discussion as it was a while, and without any issue alongside the PR, nor a description about it I really didn't think to go to DMs for the answer. Alright, that makes sense then.

Also, sorry for being inactive in this project for such a long time. I've been working on other things.

That's absolutely fine, we're all volunteers here, I also took a pretty long break from the project, but I do have some more time for it now.

Since #116 will be restructuring the entire project and will end up being a completely breaking change, I don't actually think that this breaking change needs to wait. Semantic versioning does allow breaking changes in minor version releases when <1.0.0. So, unless there's something else this needs doing, it can actually probbaly be merged already, and will be in the next (0.4.0) release.

[ItsDrike]

LGTM. I will ask you to do a rebase (or at least a regular merge, though a rebase would be cleaner) with the main branch first though, as there were a lot of changes since. There shouldn't be any breaking changes atm, so it should be as simple as git rebase main hopefully.

[Martysh12]

Looks like I messed up somewhere...

[ItsDrike]

Looks like I messed up somewhere...

No worries, I've performed the rebase for you and force-pushed. However it does seem like there were some changes in main which do affect your original chanegs (I've added tests for login state packets, which doesn't yet have the rename), causing the tests to fail.

You'll now want to do a force pull (git pull --force), overwriting your own local history with the one I pushed, and then create a new commit, renaming the login tests to also use the new mcproto.packets.packet, rather than mcproto.packets.abc (and perhaps in other places if there's more).