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

[MDB Ignore] Manifest Destiny - The Final Tile Flattening #74169

Merged
merged 29 commits into from Mar 24, 2023

Conversation

san7890
Copy link
Member

@san7890 san7890 commented Mar 22, 2023

WARNING: Do not port this script, it's a bit broken. Please get the most up to date version (see #74250) for more details.

Alt Title: The End Of The 12 Month War

About The Pull Request

Hey! Listen! This PR will cause a merge conflict with your PR! Please ensure that you have the knowledge on how to handle merge conflicts, found here: https://hackmd.io/@tgstation/ry4-gbKH5#Assured-Merge-Conflict-Resolution

Supercedes #74023 entirely.

Port of the tooling introduced in BeeStation/BeeStation-Hornet#7970 (we already had everything else), modified to meet /tg/'s requisites and culling anything that was not entirely relevant (that I could see). It's not the end of the world if I missed something tbh. Some aspects were commented out since they may be relevant to downstreams who port this PR or to enable (what I see to be) un-necessary warnings.

This is a culmination of a year's efforts, starting with Red Rover, Four Corners (#65290) and later Opposing Corners (#65455). If you don't understand why this PR exists or why it's necessary, I recommend reading both of those.

Since then, several mappers (both in their own mapping as well as tailored PRs) have worked on "flattening" out these tile turfs, however I've continually wanted a function that would mass automate it (outlined here https://tgstation13.org/phpBB/viewtopic.php?t=31872 - This functionality might still be useful if added to UpdatePaths or another type of script thereof, but I no longer have reason to keep the bounty up).

It's finally here! Yippie! A new python file, courtesy of itsmeow at BeeStation. Very awesome. As previously mentioned, a lot of alterations had to be made for our mapping desires, but the results are quite agreeable. There's a few assertions that this file makes that I had to address:

  • We have "colorless" tile decals. These are transparent, so they don't do anything. By default, bee would make these "white tiles", but we have no such thing. I decided to just add a maplint and an UpdatePaths to guard against this silliness (only Delta and Tram) had it.
  • For some reason, it labels already-converted decals with the default direction as an error state. I might touch this up in the coming hours, but for now I surpressed the error due to how many false warnings it was spitting out.

There's a few ways this tool can be improved, but I lack the knowledge on how to do so:

  • Make it so that we can run the map merger to fix the keys of the map in the update_map function, rather than run the fixer-upper python file. We can live without this to be honest. It's actually slightly good because it forces you to look at all of the MapMerge Warnings, and you can ascertain any potential errors without it silently passing you by and hitting the repository (or at least those that we haven't linted for yet).
  • Be able to pass in any regex to "flatten" anything. That's way out of scope for what I want to do here though.

How do you use this tool?

I made a readme. https://github.com/tgstation/tgstation/blob/363852cb17fa46dad8fd20e261f8f665f3e008bb/tools/MapTileAggregator/readme.md

Mapping March

oh hey it's pretty neat that this PR came out in mapping march, what a nice qol for mappers as the month enters the home stretch. ckey is san7890

Why It's Good For The Game

slimmer DMM files, better mapping practices. cool new tool. so nice.

Changelog

Nothing that really affects players, but a short summary for all those reading this PR:

  • All "corner" turf decals are flattened, and there's now a tool that we store that you can re-run to keep stuff flat in case you like mapping one way and want to fix it at the end.
  • We (should) now lint against useless uncolored turf decals since that was completely garbo as far as our codebase is concerned.
  • UpdatePaths for fixing up uncolored turf decals, yippie!

If you want to review this PR, may I suggest the file filter. You don't need to look at any of the DMM files I already did:
image

@san7890 san7890 added Code Improvement Code is now easier to copy paste. Continuous Integration Doing something stupid on every commit is supposed to be good practice now labels Mar 22, 2023
@san7890 san7890 added this to the Mega-Mega-Map-Reworks milestone Mar 22, 2023
@tgstation-server tgstation-server added Tools We pretend to be a real development community Map Edit Thank you for your tile-placing service. It's always appreciated. labels Mar 22, 2023
@san7890
Copy link
Member Author

san7890 commented Mar 22, 2023

@itsmeow Thank you again for letting me know (I didn't ping you in the body of this PR so you don't get a mention every time a downstream picks this up). Sorry I had to cull a bunch of your work, but it simply just didn't seem necessary to what we needed here? let me know if i fucked up and did a no-no but it all looks good from my end

@Maurukas
Copy link
Member

This PR will cause a merge conflict with your PR

No kidding.

This hasn't entirely sunk into my brain yet, but if I'm understanding correctly, this automates replacing multiple decals on a tile, with a single decal which accomplishes the same thing. It automates replacing the left, with the right:
image

Is that correct?

If that's the case, do we just want this running on every map related commit? Or is linting for it and telling people to manually run it preferable for some reason?

@san7890
Copy link
Member Author

san7890 commented Mar 22, 2023

Is that correct?

Yes.

If that's the case, do we just want this running on every map related commit? Or is linting for it and telling people to manually run it preferable for some reason?

Linting against it feels like an un-necessary chore (where would you even start without a two-hundred line long maplint yml or something). Realistically this doesn't do much as far as init, it's just a much cleaner way to map as well as keep a DMM file slim. The major concern of the paths rework in Red Rover was that people wouldn't be aware of such tools existing and keep mapping "the old way", but this has largely dissipated by now and will likely be extinct.

I think that now that the tool has a permanent place in the repository with the readme, if we see someone mapping the "old way", we can ask them to run the bat file (or they can do it of their own time-wasting volition). I don't see much of any value in having it run automatically like git hooks or something given how infrequently this will need to be ran (maybe once a year to keep things alright?).

@itsmeow
Copy link
Contributor

itsmeow commented Mar 22, 2023

If that's the case, do we just want this running on every map related commit? Or is linting for it and telling people to manually run it preferable for some reason?

Downstream we've had new maps, none of them have been doing it the old way, so it will just be a thing that's run occasionally if something is found.

tools/MapTileAggregator/__main__.py Outdated Show resolved Hide resolved
@itsmeow
Copy link
Contributor

itsmeow commented Mar 23, 2023

Fun thing about 4 and 8 opposing corners.

You know how shuttles rotate when landing?

They sort of have to exist because otherwise they show up and start messing with the way the floor looks. On bee they're these dumb lennyface things so if you land a shuttle the wrong way it looks very off.

This might present an issue because as it is, 4 and 8s will be converted into 1 or 2 due to the priority order. Could cause weird rotation issues. Maybe not

@san7890
Copy link
Member Author

san7890 commented Mar 23, 2023

This might present an issue because as it is, 4 and 8s will be converted into 1 or 2 due to the priority order. Could cause weird rotation issues. Maybe not

honestly worst case scenario some tiles look bleh but i think in all instances of opposing corners being in the codebase there's not really an example where a poor rotation would be the end of the world. it could very easily be standardized to both 1 and 2 again with an updatepaths (no need to update this tool to do it)

@github-actions github-actions bot added the Merge Conflict Adding upstream files to your repo via drag and drop won't resolve conflicts label Mar 24, 2023
Jacquerel pushed a commit that referenced this pull request Mar 24, 2023
![image](https://user-images.githubusercontent.com/34697715/226807239-92f9f2b9-d5ec-4ce9-a023-2e13219aee62.png)

oh that's sexy

<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may
not be viewable. -->
<!-- You can view Contributing.MD for a detailed description of the pull
request process. -->

## About The Pull Request

Hey there, blast from the past PR format. Can you spot the issue here?


![image](https://user-images.githubusercontent.com/34697715/226803527-6554007a-07ce-4f68-aabd-a5baacd33c50.png)

Well, I did while working on #74169, and it pissed me off. This is a
really simple pattern, and the only reason why it's just copypasted (and
prone to such errors) is because no one took the time to do it properly
like this. So, I just decided to do it while I had time today. Very
nice.

I also removed all the names because there was no point in them: in all
contexts you would be operating off the typepath, the name would never
come up. like so:


![image](https://user-images.githubusercontent.com/34697715/226807007-c5531f18-64ee-4ca7-b9d8-8f1ac24f3a24.png)

memory save? it's just un-needed anyways

<!-- Describe The Pull Request. Please be sure every change is
documented or this can delay review and even discourage maintainers from
merging your PR! -->

## Why It's Good For The Game

Silly error proofing is nice! Way easier to add new colors! No need to
ctrl+c/ctrl+v everything to death anymore! whoopie

<!-- Argue for the merits of your changes and how they benefit the game,
especially if they are controversial and/or far reaching. If you can't
actually explain WHY what you are doing will improve the game, then it
probably isn't good for the game in the first place. -->

## Changelog

<!-- If your PR modifies aspects of the game that can be concretely
observed by players or admins you should add a changelog. If your change
does NOT meet this description, remove this section. Be sure to properly
mark your PRs to prevent unnecessary GBP loss. You can read up on GBP
and it's effects on PRs in the tgstation guides for contributors. Please
note that maintainers freely reserve the right to remove and add tags
should they deem it appropriate. You can attempt to finagle the system
all you want, but it's best to shoot for clear communication right off
the bat. -->

Nothing that players should fret about.

<!-- Both 🆑's are required for the changelog to work! You can put
your name to the right of the first 🆑 if you want to overwrite your
GitHub username as author ingame. -->
<!-- You can use multiple of the same prefix (they're only used for the
icon ingame) and delete the unneeded ones. Despite some of the tags,
changelogs should generally represent how a player might be affected by
the changes rather than a summary of the PR's contents. -->
@tgstation-server tgstation-server removed the Merge Conflict Adding upstream files to your repo via drag and drop won't resolve conflicts label Mar 24, 2023
@ZephyrTFA ZephyrTFA enabled auto-merge (squash) March 24, 2023 19:38
@ZephyrTFA ZephyrTFA merged commit 3156a04 into tgstation:master Mar 24, 2023
16 checks passed
github-actions bot added a commit that referenced this pull request Mar 24, 2023
Jolly-66 added a commit to TaleStation/TaleStation that referenced this pull request Mar 25, 2023
…Flattening (#5048)

Original PR: tgstation/tgstation#74169
-----
Alt Title: The End Of The 12 Month War
## About The Pull Request

### Hey! Listen! This PR _will_ cause a merge conflict with your PR!
Please ensure that you have the knowledge on how to handle merge
conflicts, found here:
https://hackmd.io/tgstation/ry4-gbKH5#Assured-Merge-Conflict-Resolution

Supercedes #74023 entirely.

Port of the tooling introduced in
BeeStation/BeeStation-Hornet#7970 (we already
had everything else), modified to meet /tg/'s requisites and culling
anything that was not entirely relevant (that I could see). It's not the
end of the world if I missed something tbh. Some aspects were commented
out since they may be relevant to downstreams who port this PR or to
enable (what I see to be) un-necessary warnings.

This is a culmination of a year's efforts, starting with _Red Rover,
Four Corners_ (#65290) and later _Opposing Corners_ (#65455). If you
don't understand why this PR exists or why it's necessary, I recommend
reading both of those.

Since then, several mappers (both in their own mapping as well as
tailored PRs) have worked on "flattening" out these tile turfs, however
I've continually wanted a function that would mass automate it (outlined
here https://tgstation13.org/phpBB/viewtopic.php?t=31872 - This
functionality might still be useful if added to UpdatePaths or another
type of script thereof, but I no longer have reason to keep the bounty
up).

It's finally here! Yippie! A new python file, courtesy of itsmeow at
BeeStation. Very awesome. As previously mentioned, a lot of alterations
had to be made for our mapping desires, but the results are quite
agreeable. There's a few assertions that this file makes that I had to
address:

* We have "colorless" tile decals. These are transparent, so they don't
do anything. By default, bee would make these "white tiles", but we have
no such thing. I decided to just add a maplint and an UpdatePaths to
guard against this silliness (only Delta and Tram) had it.
* For some reason, it labels already-converted decals with the default
direction as an error state. I might touch this up in the coming hours,
but for now I surpressed the error due to how many false warnings it was
spitting out.

There's a few ways this tool can be improved, but I lack the knowledge
on how to do so:
* Make it so that we can run the map merger to fix the keys of the map
in the `update_map` function, rather than run the fixer-upper python
file. We can live without this to be honest. It's actually slightly good
because it forces you to look at all of the MapMerge Warnings, and you
can ascertain any potential errors without it silently passing you by
and hitting the repository (or at least those that we haven't linted for
yet).
* Be able to pass in any regex to "flatten" anything. That's way out of
scope for what I want to do here though.

## How do you use this tool?

I made a readme.
https://github.com/tgstation/tgstation/blob/363852cb17fa46dad8fd20e261f8f665f3e008bb/tools/MapTileAggregator/readme.md
### Mapping March
oh hey it's pretty neat that this PR came out in mapping march, what a
nice qol for mappers as the month enters the home stretch. ckey is
san7890

## Why It's Good For The Game

slimmer DMM files, better mapping practices. cool new tool. so nice.
## Changelog
Nothing that really affects players, but a short summary for all those
reading this PR:

* All "corner" turf decals are flattened, and there's now a tool that we
store that you can re-run to keep stuff flat in case you like mapping
one way and want to fix it at the end.
* We (should) now lint against useless uncolored turf decals since that
was completely garbo as far as our codebase is concerned.
* UpdatePaths for fixing up uncolored turf decals, yippie!


If you want to review this PR, may I suggest the file filter. You don't
need to look at any of the DMM files I already did:

![image](https://user-images.githubusercontent.com/34697715/226787961-ab82cad4-5d6d-4788-a7bd-5071aac825c4.png)

---------

Co-authored-by: san7890 <the@san7890.com>
Co-authored-by: Jolly-66 <70232195+Jolly-66@users.noreply.github.com>
scriptis pushed a commit to scriptis/tgstation that referenced this pull request Apr 1, 2023
![image](https://user-images.githubusercontent.com/34697715/226807239-92f9f2b9-d5ec-4ce9-a023-2e13219aee62.png)

oh that's sexy

<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may
not be viewable. -->
<!-- You can view Contributing.MD for a detailed description of the pull
request process. -->

## About The Pull Request

Hey there, blast from the past PR format. Can you spot the issue here?


![image](https://user-images.githubusercontent.com/34697715/226803527-6554007a-07ce-4f68-aabd-a5baacd33c50.png)

Well, I did while working on tgstation#74169, and it pissed me off. This is a
really simple pattern, and the only reason why it's just copypasted (and
prone to such errors) is because no one took the time to do it properly
like this. So, I just decided to do it while I had time today. Very
nice.

I also removed all the names because there was no point in them: in all
contexts you would be operating off the typepath, the name would never
come up. like so:


![image](https://user-images.githubusercontent.com/34697715/226807007-c5531f18-64ee-4ca7-b9d8-8f1ac24f3a24.png)

memory save? it's just un-needed anyways

<!-- Describe The Pull Request. Please be sure every change is
documented or this can delay review and even discourage maintainers from
merging your PR! -->

## Why It's Good For The Game

Silly error proofing is nice! Way easier to add new colors! No need to
ctrl+c/ctrl+v everything to death anymore! whoopie

<!-- Argue for the merits of your changes and how they benefit the game,
especially if they are controversial and/or far reaching. If you can't
actually explain WHY what you are doing will improve the game, then it
probably isn't good for the game in the first place. -->

## Changelog

<!-- If your PR modifies aspects of the game that can be concretely
observed by players or admins you should add a changelog. If your change
does NOT meet this description, remove this section. Be sure to properly
mark your PRs to prevent unnecessary GBP loss. You can read up on GBP
and it's effects on PRs in the tgstation guides for contributors. Please
note that maintainers freely reserve the right to remove and add tags
should they deem it appropriate. You can attempt to finagle the system
all you want, but it's best to shoot for clear communication right off
the bat. -->

Nothing that players should fret about.

<!-- Both 🆑's are required for the changelog to work! You can put
your name to the right of the first 🆑 if you want to overwrite your
GitHub username as author ingame. -->
<!-- You can use multiple of the same prefix (they're only used for the
icon ingame) and delete the unneeded ones. Despite some of the tags,
changelogs should generally represent how a player might be affected by
the changes rather than a summary of the PR's contents. -->
scriptis pushed a commit to scriptis/tgstation that referenced this pull request Apr 1, 2023
…74169)

Alt Title: The End Of The 12 Month War
## About The Pull Request

### Hey! Listen! This PR _will_ cause a merge conflict with your PR!
Please ensure that you have the knowledge on how to handle merge
conflicts, found here:
https://hackmd.io/@tgstation/ry4-gbKH5#Assured-Merge-Conflict-Resolution

Supercedes tgstation#74023 entirely.

Port of the tooling introduced in
BeeStation/BeeStation-Hornet#7970 (we already
had everything else), modified to meet /tg/'s requisites and culling
anything that was not entirely relevant (that I could see). It's not the
end of the world if I missed something tbh. Some aspects were commented
out since they may be relevant to downstreams who port this PR or to
enable (what I see to be) un-necessary warnings.

This is a culmination of a year's efforts, starting with _Red Rover,
Four Corners_ (tgstation#65290) and later _Opposing Corners_ (tgstation#65455). If you
don't understand why this PR exists or why it's necessary, I recommend
reading both of those.

Since then, several mappers (both in their own mapping as well as
tailored PRs) have worked on "flattening" out these tile turfs, however
I've continually wanted a function that would mass automate it (outlined
here https://tgstation13.org/phpBB/viewtopic.php?t=31872 - This
functionality might still be useful if added to UpdatePaths or another
type of script thereof, but I no longer have reason to keep the bounty
up).

It's finally here! Yippie! A new python file, courtesy of itsmeow at
BeeStation. Very awesome. As previously mentioned, a lot of alterations
had to be made for our mapping desires, but the results are quite
agreeable. There's a few assertions that this file makes that I had to
address:

* We have "colorless" tile decals. These are transparent, so they don't
do anything. By default, bee would make these "white tiles", but we have
no such thing. I decided to just add a maplint and an UpdatePaths to
guard against this silliness (only Delta and Tram) had it.
* For some reason, it labels already-converted decals with the default
direction as an error state. I might touch this up in the coming hours,
but for now I surpressed the error due to how many false warnings it was
spitting out.

There's a few ways this tool can be improved, but I lack the knowledge
on how to do so:
* Make it so that we can run the map merger to fix the keys of the map
in the `update_map` function, rather than run the fixer-upper python
file. We can live without this to be honest. It's actually slightly good
because it forces you to look at all of the MapMerge Warnings, and you
can ascertain any potential errors without it silently passing you by
and hitting the repository (or at least those that we haven't linted for
yet).
* Be able to pass in any regex to "flatten" anything. That's way out of
scope for what I want to do here though.

## How do you use this tool?

I made a readme.
https://github.com/tgstation/tgstation/blob/363852cb17fa46dad8fd20e261f8f665f3e008bb/tools/MapTileAggregator/readme.md
### Mapping March
oh hey it's pretty neat that this PR came out in mapping march, what a
nice qol for mappers as the month enters the home stretch. ckey is
san7890

## Why It's Good For The Game

slimmer DMM files, better mapping practices. cool new tool. so nice.
## Changelog
Nothing that really affects players, but a short summary for all those
reading this PR:

* All "corner" turf decals are flattened, and there's now a tool that we
store that you can re-run to keep stuff flat in case you like mapping
one way and want to fix it at the end.
* We (should) now lint against useless uncolored turf decals since that
was completely garbo as far as our codebase is concerned.
* UpdatePaths for fixing up uncolored turf decals, yippie!


If you want to review this PR, may I suggest the file filter. You don't
need to look at any of the DMM files I already did:

![image](https://user-images.githubusercontent.com/34697715/226787961-ab82cad4-5d6d-4788-a7bd-5071aac825c4.png)

---------

Co-authored-by: Zephyr <12817816+ZephyrTFA@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Improvement Code is now easier to copy paste. Continuous Integration Doing something stupid on every commit is supposed to be good practice now Map Edit Thank you for your tile-placing service. It's always appreciated. Tools We pretend to be a real development community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants