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

[CR] New Town Hall Variant #54554

Conversation

JonathanLochridge
Copy link
Contributor

@JonathanLochridge JonathanLochridge commented Jan 18, 2022

Summary

Content "New Town Hall Variant"

Purpose of change

I thought having more town hall variants would be nice. Also that it might be a relatively easy thing to change for a first location change.

Describe the solution

Changed the classroom on floor 2 to a set of conference rooms.
Made the windows more symmetrical.

Describe alternatives you've considered

-A variant with an included small library.
-A variant with an attached greenspace and parking lot.

Testing

All files Linted.
New variant can spawn in latest experimental when these files are added.
Error occurs on startup but I think it is unrelated to the new variant.

Additional context

It could have some minor items added to the new room. Trying to figure out how to do that. Now that I got it to spawn properly I should work on that and adding it to the start location for the town hall on my next pass after this PR.
Functional

Added four confrence rooms where the classroom was and made some windows bigger for a varient?
I want to use a nested map to make it easier to understand later and have less wasted file space.
I also still need to hook the varient into mapgen
Adding new Town_Hall Variant Info to other files
Should be able to spawn naturally?
Also, cleaned up the town hall to remove the redundant roof from the file.
Doesn't fix error but does change a few minor style things.
@github-actions github-actions bot added the astyled astyled PR, label is assigned by github actions label Jan 19, 2022
Doesn't fix the issue on new character.
@JonathanLochridge
Copy link
Contributor Author

(I thought I already posted this but I forgot to.)
The following error occurs when I run the 2nd newest experimental with the included changes. Then it gets stuck while trying to open up the create characters screen on the inventory items point. Even though I didn't touch that.
When I hit play now it sometimes crashes instead of freezing.
The same error occurs for all of the new tile maps for the new town hall.

DEBUG : Mapgen town_hall_2_0_0_0 is not used by anything!

FUNCTION : void mapgen_factory::check_consistency()
FILE : src/mapgen.cpp
LINE : 435
VERSION : f1997bf

@LovamKicsiGazsii
Copy link
Contributor

Since you didn't change the town hall too drastically, I think duplicating the entire building is a bit overkill. If you only want to make a variant of one om_terrain, you could take advantage of the weight system and assign two mapgens on the same overmap terrain. That way, you wouldn't have to edit regional_map_settings.json, multitile_city_buildings.json, and overmap_terrain_public_institutional.json.

Btw, nice addition!

@JonathanLochridge
Copy link
Contributor Author

@LovamKicsiGazsii I am not exactly sure how to do that? My biggest change is the the second story but I had small changes to the first floor. I thought about trying to split up the given map so that I only had to include the changed sections. But, I couldn't even get it to work together. And If I cut it out at the wrong point it would probably mess it up.
I have a couple more ambitious ideas. But, I wanted to make sure I could get a simpler one to work first.

It still doesn't work. Not sure why it crashes? I can't figure out why?
It would be nice if someone was willing to help me figure out why. But, I don't expect anyone to. So for now I have been working on other stuff.

@JonathanLochridge
Copy link
Contributor Author

JonathanLochridge commented Jan 28, 2022

I did some checking and apparently there aren't any crashes on the newest experimental version and it can be debug spawned properly but it doesn't seem to naturally occur at normal spawn rates. Although, maybe I was just unlucky? Since when I ramped up the spawn rate to a higher level it spawned without error.
I do get an error on new game but it seems to be a different one and doesn't impede spawning.
I also want to add the new varient to the list of spawn options for the town hall start. however, Since I forked this branch before that change was merged it might be easier to put that in it's own PR?
The debug line below doesn't seem to to be the same coordinates as where the new town hall varient spawned so it may be unrelated. If someone could give me an idea if this error occurs sometimes without my change then I think it should be close to ready to merge.
I am unsure why the JSON check said I had an issue though? I checked the log and it didn't seem to have any particular area it was pointing out as a problem? And I ran everything through a linter? Maybe if it is run again it will go away?

DEBUG : loading non-main map at (462,610,0) which overlaps with main map (abs_sub = (459,607,0)) is not supported

FUNCTION : void map::load(const tripoint_abs_sm&, bool, bool)
FILE : src/map.cpp
LINE : 6710
VERSION : 761fb51

After some more testing I am pretty sure it's a local issue. Since someone else tested it and it didn't occur for them? Or maybe it's an OS specific issue that has nothing to do with this?

@JonathanLochridge JonathanLochridge changed the title [CR][WIP] New Town Hall Variant [CR] New Town Hall Variant Jan 28, 2022
@JonathanLochridge JonathanLochridge marked this pull request as ready for review January 28, 2022 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants