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

Palette overhaul; replaces the old hardcoded numeric id based palette … #308

Merged
merged 2 commits into from
Dec 20, 2020

Conversation

evildeeds
Copy link
Collaborator

@evildeeds evildeeds commented Dec 13, 2020

Palette overhaul; replaces the old hardcoded numeric id based palette with the modern namespaced palette loaded from an external json file. For issues #301 #303 #307

Now I have seen that @tanaka141 have been working on supporting the map format; this patch does only deal with the internal palette to support modern versions of Minecraft, it does not fix any missing blocks or handle the new map format itself, but does enable editing the palette without needing to recompile.

Since this is a rather large change I'm pushing it as a pull request for review.

…with the modern namespaced palette loaded from an external json file.
@evildeeds evildeeds marked this pull request as draft December 13, 2020 13:35
Fixed the deprected color override from command line feature
A minor grammar correction
@evildeeds evildeeds marked this pull request as ready for review December 15, 2020 07:30
@evildeeds
Copy link
Collaborator Author

Well; I think that will have to do, I'll wait a bit longer for any feedback before merging and in the meantime I'll have a go at trying to handle the new map format (I suppose it isn't that new any longer..).

As for a code clean up run which kind of is needed, that will have to wait for a bit longer.

@udoprog
Copy link
Owner

udoprog commented Dec 17, 2020

Well; I think that will have to do, I'll wait a bit longer for any feedback before merging and in the meantime I'll have a go at trying to handle the new map format (I suppose it isn't that new any longer..).

As for a code clean up run which kind of is needed, that will have to wait for a bit longer.

Kudos for any work you want to do! Just make sure it's on your own behest and you think it's fun. I have other projects I've moved on to!

FWIW, on the surface the patch looks fine to me!

@evildeeds evildeeds changed the title Palette overhaul; replaces the old harcoded numeric id based palette … Palette overhaul; replaces the old hardcoded numeric id based palette … Dec 19, 2020
@evildeeds evildeeds merged commit 15d0bfe into udoprog:master Dec 20, 2020
@evildeeds
Copy link
Collaborator Author

evildeeds commented Dec 22, 2020

Well; I think that will have to do, I'll wait a bit longer for any feedback before merging and in the meantime I'll have a go at trying to handle the new map format (I suppose it isn't that new any longer..).
As for a code clean up run which kind of is needed, that will have to wait for a bit longer.

Kudos for any work you want to do! Just make sure it's on your own behest and you think it's fun. I have other projects I've moved on to!

FWIW, on the surface the patch looks fine to me!

Let me put it this way; I have two reasons for picking this project up again, first Minecraft somehow became relevant once again and I also felt that I need to refresh my C++. And so what better way is there but to pick this back up as quite a bit of maintenance is required before it can work with newer maps. I'm currently working on consolidating block lookup code from the rendering decision switch-blocks and into level handling code, which I think is necessary to prime the code base for handling a second map type.

Edit: There is also a situation related to an ongoing epidemic meaning that I'm going nowhere, so to avoid being completely bored out of my mind during these holidays...

@evildeeds evildeeds deleted the palette-overhaul branch December 22, 2020 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants