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

Update buildings.ts #316

Closed
wants to merge 4 commits into from
Closed

Update buildings.ts #316

wants to merge 4 commits into from

Conversation

Leon-9982
Copy link

Removed original center lootspawner and added two to the entrances.
Removed all small boxes as they cramped the space too much.
Readjusted location of ammo crate and applied a 0.97 scale over it to make some space around them.
Added a regular crate to the center.
Remade PR because I forgot to add the center regular crate.
image

P.S Check code for errors.

bien-star and others added 3 commits May 18, 2024 10:53
Removed original center lootspawner and added two to the entrances.
Removed all small boxes as they cramped the space too much.
Readjusted location of ammo crate and applied a 0.97 scale over it to make some space around them.
Added a regular crate to the center.
@Leon-9982 Leon-9982 requested a review from hsanger as a code owner May 24, 2024 15:24
@1092384
Copy link
Contributor

1092384 commented May 24, 2024

you know you can just push more commits to the same pr

@Leon-9982
Copy link
Author

mb didnt know

@bien-star
Copy link
Contributor

Did you also move and scale the hitboxes of the ammo crates?

@bien-star
Copy link
Contributor

Also why did you remove the boxes

@Leon-9982
Copy link
Author

It looks too messy with them there, I've added two loot spawners to compensate for them

@Leon-9982
Copy link
Author

Did you also move and scale the hitboxes of the ammo crates?

When I add the sclae property, it adjusts the size and hitboxes of the object, so yes.

@bien-star
Copy link
Contributor

It looks too messy with them there, I've added two loot spawners to compensate for them
The loot spawner table is completely different than the box loot pool, so this throws the balance off considerably. Also, what's your rationale for adding another crate?

@Leon-9982
Copy link
Author

It looks too messy with them there, I've added two loot spawners to compensate for them
The loot spawner table is completely different than the box loot pool, so this throws the balance off considerably. Also, what's your rationale for adding another crate?

The two spawners uses the same loottable as the one that used to be in the center of the warehouse before, i added two to compensate for the removal of the small boxes, also what other crate?

@Err0r430
Copy link
Collaborator

Farming commits be like

@bien-star
Copy link
Contributor

The boxes that are in the warehouse use the box loot table, whereas you added another warehouse tier spawner which is completely different. Also, there is now a middle box which wasn’t there before. Overall, there’s a lot better loot available now. What’s the reasoning for this change?

@bien-star
Copy link
Contributor

Also that’s an insanely small space between the ammo crate and the wall, even with it scaled down. I don’t think this is gonna work

@Leon-9982
Copy link
Author

Leon-9982 commented May 24, 2024

The boxes that are in the warehouse use the box loot table, whereas you added another warehouse tier spawner which is completely different. Also, there is now a middle box which wasn’t there before. Overall, there’s a lot better loot available now. What’s the reasoning for this change?

Added the center box because I wanted to basically, cuz it felt empty without it, and I didn't know what other tables existed so just used the existing one. @hsanger can edit the spawner loot table if he wishes so.

@Leon-9982
Copy link
Author

Also that’s an insanely small space between the ammo crate and the wall, even with it scaled down. I don’t think this is gonna work

nah it's fine, while testing I could move quickly through there, and besides you can destroy the reg crate.

@Leon-9982
Copy link
Author

I don't mind removing the loot spawners.

@bien-star
Copy link
Contributor

bien-star commented May 24, 2024

Added the center box because I wanted to basically, cuz it felt empty without it, and I didn't know what other tables existed so just used the existing one. @hsanger can edit the spawner loot table if he wishes so

If you want the spawner table changed, do it yourself.
And adding things just because it feels empty isn’t really a good justification. Why don’t you try to find a way to add cover to the warehouses that doesn’t necessitate completely changing the loot?

@emeraldn30
Copy link

also there shouldn't be a 0.97 size multiplier on the ammo crates if you can help it imo

@Compositr
Copy link
Collaborator

Yeah be careful with scaling down stuff cause it messes up the breaking scaling

@kaklikOf13
Copy link
Contributor

I Like

Copy link
Collaborator

@Compositr Compositr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L#371-372 Scaling down is not optimal as it messes with the breaking animation.

Removed scale property on ammo crates in warehouse and loot table adjusted to regular_crate, removed the central reg_crate and moved the ammocrates closer to the centre of warehouse
@Leon-9982
Copy link
Author

L#371-372 Scaling down is not optimal as it messes with the breaking animation.

I've removed scaling, and did some other stuff in my newest commit @Compositr

Copy link
Collaborator

@Compositr Compositr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@bien-star
Copy link
Contributor

If we're going to go with this maybe it could be one of two possible layouts? Randomly either this one or the original.
Also hopefully once there's some more buildings on the map we can switch to fewer and larger warehouses rather than these dollar store ones.

@Leon-9982 Leon-9982 closed this by deleting the head repository Jun 24, 2024
@1092384
Copy link
Contributor

1092384 commented Jul 2, 2024

lmfao

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.

8 participants