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

Forward port mushroom visual-only change #4214

Open
jostephd opened this issue Aug 11, 2019 · 7 comments

Comments

@jostephd
Copy link
Member

commented Aug 11, 2019

Forward port #4185 to make the ^Uf code's graphics match its behavior. For adding a new terrain code for true mixed terrain fungus, see #4213.

@doofus-01

This comment has been minimized.

Copy link
Member

commented Aug 11, 2019

The old behavior is pretty much a mistake, don't we just want to clean it up in 1.15, rather than forward-porting the band-aid? Or is that breaking a deprecation cycle? We can make a dummy terrain ^Uf so that old maps can still be opened.

@soliton-

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

The current mushroom terrain is not a mistake regarding map balance. It was visually misleading though.

@slavrenyuk

This comment has been minimized.

Copy link

commented Aug 12, 2019

Port this PR as well #4216 please.

@jostephd

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

@doofus-01 The ^Uf code should be preserved for compatibility, but right now on master it still suffers from the problem you described in #4185 (comment). See also #4213 if you haven't already. I think we're saying the same thing, aren't we? Or am I misunderstanding you?

@slavrenyuk We only create "forward port" issues for things that have already been merged to 1.14. You don't need to separately ask for PRs to be merged to master unless they have been merged to 1.14 or have the green "Stable 1.14" label.

@slavrenyuk

This comment has been minimized.

Copy link

commented Aug 13, 2019

@slavrenyuk We only create "forward port" issues for things that have already been merged to 1.14. You don't need to separately ask for PRs to be merged to master unless they have been merged to 1.14 or have the green "Stable 1.14" label.

Ok, thanks for the information.

@doofus-01

This comment has been minimized.

Copy link
Member

commented Aug 18, 2019

@jostephd - I think we may have been saying the same thing. My point was that the (now reverted) PR was an attempt at fixing the issue in a way that didn't change the actual behavior, but it wasn't the best way to deal with things going forward:

  • The existing graphics and terrain codes said it was an overlay (implying mixed terrain), the actual terrain wasn't mixed terrain.
  • 1.15 allows new terrain codes, so there is a new mushroom terrain that really is an overlay/mixed terrain (*^To, or whatever), and a new one that is a base terrain (both graphics and terrain code, Tb or whatever), maintaining the old behavior but with a more sensible terrain code -> 1:1 replacement in updating maps. Also, as a base terrain, "Tb" could have other overlays.
  • there is no reason to maintain ^Uf in 1.15, except as a deprecated old code to keep maps from crashing. The new terrains should not be ^Uf.

That may have been hashed out in the other issues and PRs, I was out of the loop for a bit. I read enough in the discord logs to not want to touch this (or anything) anymore for now, but if you need help later on in 1.15 with the terrain graphics, I can pitch in when the mechanics are settled.

@slavrenyuk

This comment has been minimized.

Copy link

commented Aug 19, 2019

This issue is not trivial, it requires attention of many people. Especially those who did not participate in the discussion from the beginning, namely @nemaara @shikadiqueen and @Vultraz. IMO it doesn't make sense to work on implementation, until there is a plan that satisfies all parties. In any case, thanks to @doofus-01 we already have new graphics, which is usually hard to get. If it will be decided to keep status quo, I would ask to add this information to list of proposed and rejected ideas (on the forums).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.