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

Heavy mining mod added. Add a mining car. #34229

Merged
merged 16 commits into from
Oct 1, 2019

Conversation

Ckpyt
Copy link
Contributor

@Ckpyt Ckpyt commented Sep 25, 2019

Summary
SUMMARY: Mods "Heavy mining mod added. Add a mining car"

Purpose of change
In my mod, I added a vehicle part, that can crush mineable tiles in 1 tile around it. Plus, added a roadheader with this part, that can be spawned in junkyards, industrial sites or as a workshop vehicle.
Also added a balancer for making balance.

Describe the solution
The possibility of mining with heavy vehicles(because of wight).

Describe alternatives you've considered
The question is: how all underground objects were created? CDDA has only not so effective tools or robots. Any small towns could have subways, laboratory or other objects. So, it means, CDDA should have some more effective heavy mining vehicles from the real world. It is a first step for making it. In future plans, I have mining shields for making subway tunnels, tunnel boring machines and so on...
Additional context

Concept:
Roadheader: https://en.wikipedia.org/wiki/Roadheader,
https://tunnelingonline.com/buying-a-roadheader-heres-what-to-consider/

@Fris0uman
Copy link
Contributor

Fris0uman commented Sep 25, 2019

You need to lint your json files
http://dev.narc.ro/cataclysm/format.html
Also please follow the PR template

@KorGgenT KorGgenT added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding labels Sep 25, 2019
@KorGgenT
Copy link
Member

you also need to astyle your C++

@Ckpyt
Copy link
Contributor Author

Ckpyt commented Sep 25, 2019

you also need to astyle your C++

That I should change?

Is JSON good now?

@KorGgenT
Copy link
Member

you're going to need to revert the changes you made to the gitignore and the VS project. there's an astyle plugin for VS that'll give you the astyle rules we use on our project; i can give you more details if you need them in a while.

@Fris0uman
Copy link
Contributor

Fris0uman commented Sep 25, 2019

See: https://github.com/CleverRaven/Cataclysm-DDA/blob/master/doc/CODE_STYLE.md for code style guidelines

}
}
}
if (crush_target.z != -OVERMAP_LAYERS) { //target chosen
Copy link
Contributor

Choose a reason for hiding this comment

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

OVERMAP_LAYERS equals to 1 + OVERMAP_DEPTH + OVERMAP_HEIGHT. You probably need to use -OVERMAP_DEPTH here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, I'm checking the value, that never could exist for understanding did I found something or not. As you can understand, any rightful value could not be below -OVERMAP_DEPTH. So, I can use -OVERMAP_DEPTH - 1 or -(OVERMAP_DEPTH + 1) or -(1 + OVERMAP_DEPTH + OVERMAP_HEIGHT).

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I started reading code from the bottom, but now see what you mean. I think you better use something else here to avoid confusion - maybe use one of inbounds function that we have?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is better. Thanks for help ;-)

@@ -1092,6 +1093,38 @@ void vehicle::play_chimes()
}
}

void vehicle::crash_terrain_around() {
const tripoint around[] = { {1, 1, 0}, { 0,1,0 }, {-1, 1, 0}, { 1, 0, 0}, {0, 0, 0}, { -1, 0, 0}, {1, -1, 0}, {0, -1, 0}, {-1, -1, 0} };
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a collection of neighboring tripoints in

static const std::array<tripoint, 8> eight_horizontal_neighbors = { {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. Thanks, I've fixed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you help me: that is wrong in my code?
Because I have no idea why is test fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

Failing test is unrelated to your changes it probably caught #33640

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and how I can fix it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. I've merged my branch with the newest changes. Maybe, it could help.

@@ -1092,6 +1093,38 @@ void vehicle::play_chimes()
}
}

void vehicle::crash_terrain_around() {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is void vehicle::transform_terrain() function which already does something like crash_terrain_around does - maybe you can combine it with your code to avoid code duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately, not. I think about it, but it has a lot of differences.

@ZhilkinSerg
Copy link
Contributor

FYI on using Astyle in VS: #24006

@Rail-Runner
Copy link
Contributor

Any chance for this to be mainline content? Well, at least the vehicle parts until there's a proper support for tracks.

@Ckpyt
Copy link
Contributor Author

Ckpyt commented Sep 26, 2019

Any chance for this to be mainline content? Well, at least the vehicle parts until there's a proper support for tracks.

The part has a big mass and using this part without tracks - is a really bad idea. Also, wheels cannot live a long time in a mine - its destroyed after 5-10 minutes of working. So, the vehicle should have tracks. Without it - there is no chance to be mainline content ;-(

@thethunderhawk
Copy link

thethunderhawk commented Sep 27, 2019

Tbh I think the player personally running a tunnel boring machine is a bit out of the scope of mainline CDDA. You’re talking about displacing thousands of tons of soil and rock. Even just digging a man-sized tunnel with a pickaxe would realistically take a lot of excavating of material (see The Great Escape), I’ve always just assumed we’re abstracting that by inflating dig times, but moving a whole subway tunnel’s worth of debris would seem to stretch the realism of that far past the limit.

As a mod I love it though. Maybe consider one of these: http://www.sheepletv.com/nuclear-tunnel-boring-machines-switzerland-has-nothing-on-us-2/

@Ckpyt
Copy link
Contributor Author

Ckpyt commented Sep 27, 2019

Tbh I think the player personally running a tunnel boring machine is a bit out of the scope of mainline CDDA. You’re talking about displacing thousands of tons of soil and rock.

It is more about the game mechanic: my mod just destroying anything around the part. So, how much soil, clay or stone - depends on a game engine. Also, it is a good mechanism to mine some ores - they really rare to mine by hands tools.

I’ve always just assumed we’re abstracting that by inflating dig times, but moving a whole subway tunnel’s worth of debris would seem to stretch the realism of that far past the limit.

It will take much more time and low fuel then now. Now, it consumes 60 litres of diesel fuel for 3 hours working. To my mind, it is good for fast-boring, but digging a tunnel should take much more time and not so much fuel. Maybe, it will be nuclear-powered ;-)

As a mod I love it though.

Thanks a lot ;-)

@Ckpyt Ckpyt requested a review from ZhilkinSerg September 28, 2019 09:28
Pervios build fails on Android build - there was not enought time for the task.
@Ckpyt
Copy link
Contributor Author

Ckpyt commented Sep 28, 2019

All the tests had passed. Before, Android build has not enough time for building, now - the last one. So, everything is good and passed but not at the same time ;-(

@ZhilkinSerg ZhilkinSerg self-assigned this Oct 1, 2019
@ZhilkinSerg ZhilkinSerg merged commit a8dfd59 into CleverRaven:master Oct 1, 2019
@ZhilkinSerg ZhilkinSerg removed their assignment Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants