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][WIP] Foundation for implementing medieval mod shields #14711

Closed
wants to merge 17 commits into from

Conversation

Projects
None yet
8 participants
@chaosvolt
Copy link
Contributor

commented Jan 4, 2016

This is one of those ideas I'd had on the backburner for a while. Basically, there are two main things preventing the inclusion of shields:

  • The big one, my original hackish solution does not allow for the equipped shield to preclude use of two-handed weapons.
  • Second, an inability to actively block with them.

The main suggestion that has come up as a way to support these features has been the idea of an off-hand to hold a shield or secondary weapon in. However, this would be a big change, and has been an oft-suggested idea generally dismissed as not worth the effort.

The main reason is that while it could be used for some refactoring of picking items up and holding them, the other main uses would be:

  • Going all John Woo with two pistols, which has been flatly dismissed as impractical.
  • Doing the same with weapons weapons, which is also impractical. Barring situational uses like rapier-and-dagger, which would STILL be of limited use because how many zombie swordsmen do you expect to encounter?

Additionally, one thing lost when changing shields in the way originally proposed is a loss of passive protection from ranged attacks, which is a major element of historical shield usage. The ability to actively block with them PARTIALLY exists via arm blocks, but one part of this PR will eventually address that.


Onto the actual content of the PR. Barring a pressing need to go through the effort of adding offhand support (which is regarded as low-priority and low-utility), adapting the existing method to work in a more plausible manner might be better for the moment.

  • First on the coding list, allowing worn items to preclude the use of both hands. The "you can't wield this thing" check should additionally print a message if one of the causes for a failed attempt to wear a two-handed weapon was due to a worn item.
  • Ported over shields from the dropbox'd version, with the flag added.
  • Added mention of the flag to item.cpp and the JSON documentation.

The other half of the required code is in #14741 it seems. Thanks to Coolthulhu for that, as my attempt at it did not go well. x_x

chaosvolt
Foundation for implementing medieval mod shields
* First on the coding list, allowing worn items to preclude the use of
both hands. The "you can't wield this thing" check should additionally
print a message if one of the causes for a failed attempt to wear a
two-handed weapon was due to a worn item.
* Ported over shields from the dropbox'd version, with the flag added.
* Added mention of the flag to item.cpp and the JSON documentation.

Potentially adding active blocking of melee attacks will be worked on
once the first main prerequisite is sorted out.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 4, 2016

Success on the first commit? Glorious.

Additionally pondering other uses for the implemented flag, via any other items that might warrant it.

@chaosvolt chaosvolt changed the title Foundation for implementing medieval mod shields [CR][WIP] Foundation for implementing medieval mod shields Jan 4, 2016

"encumbrance" : 0,
"bashing" : 8,
"symbol" : "[",
"flags" : ["OVERSIZE", "STURDY", "BELTED"],

This comment has been minimized.

Copy link
@Malkeus

Malkeus Jan 4, 2016

Contributor

Needs new flag added? and your other shields too...

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 4, 2016

Ack. I could've sworn I added the flag to those. @_@

Chaosvolt
Adds forgotten flag to shields
I either didn't save the file changes, or did the editing for the copy of the Dropbox'd version I had on hand, instead of the Github version. >.<
@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 4, 2016

I have no idea where you get the idea that off-hand support is "low-priority and low-utility". It is harder to do, but it is the correct way to handle shields and off-hand weapons (and off-hand tools). See http://smf.cataclysmdda.com/index.php?topic=10444.msg247535#msg247535

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 4, 2016

Hmm. Part of was that it hasn't been discussed much in a while. Given the complexity it would add, it likely won't be taken up by anyone competent enough to try it for quite some time.

EDIT: That reminds me. This PR would at least have the added use of adding the relevant flag to briefcases and other items.

chaosvolt
Adds flag to briefcase-type items
* Only likely non-mod use for the flag that exists.
@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

If we are ever to consider the hacky shields, they have to do at least those two things:

  • Prevent using weapons AND shields with the hand they cover. Since unarmed strike is a weapon, this leaves us with just one option: only one shield may ever be worn and the wielded weapon must be 1-handed if the shield is to be worn
  • The shield must be usable to execute blocks with. As in - it must enable the block technique to be used AND must correctly redirect the damage to the shield.

The former is simple - just check one-handedness on equip and disallow one-handed characters from wearing shields (if they want to use shields as weapons, they can wield them as weapons).

The latter wouldn't be hard, but it's not totally straightforward.
You'd have to:

  • Write a function that locates a shield in player's worn items and returns it
  • Use it in player::block_hit in melee.cpp to replace weapon if an equipped shield exists
  • Update player::handle_melee_wear to take an item argument so that things other than current weapon can be damaged
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

Hmm. The latter was something I had planned, yes. Might need to look for the equip code to see how to implement the former.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

And the latest entry in the "What was I thinking?" chronicles, started to add a check in the wear functions for "are we wearing a briefcase/shield OR do we lack hands" before remembering that I already added "are we wearing a briefcase/shield" to the function for defining whether the player has only one arm free.

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

Welcome to the department of redundancy department, you are welcome in this place!

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

Then again, if I could think up a way to say "You don't have a hand free to wear this." in two differently distinct ways, I could split that check so the message returned is more clear as to why the player can't wear it. Though then I'd need to further refine the has_two_arms check or split "wearing a briefcase/shield" in a separate check.

chaosvolt added some commits Jan 5, 2016

chaosvolt
Prevent wearing briefcase/shield with only one arm
* Adds handling of one-armed bandits trying to wear items with
RESTRICT_HANDS.  Also occurs when one's already wearing an item with
that flag.
chaosvolt
Fixed mis-copy, forgotten ()
* Incorrect bit of copy-paste.
* Error: Reference to non-static member function must be called, not
faxed. :V
chaosvolt
Fixed misplaced fix
Yes, place the correct bit of code in the wrong check, replacing
something other than the broken bit you meant to replace. So good. Very
brilliant.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

First I copied from the check that had a count (which, as you said, wasn't initialized that far up), then I wound up placing the intended replacement in the wrong place, by overwriting the part in the "you can't wear more than 2 X" by mistake. >.<

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

No worries man, this shit is confusing. At least you're trying ;)

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

...and then something still failed. Whee.

EDIT: Extraneous ) due to yet more lazy copy-paste.

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

Parentheses, brackets, braces and commas can all go to hell...

chaosvolt
Fixed extraneous )
This is what happens when you change your mind and decide you want to
inject some %s in your formerly-generic "You can't wear that." message.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

That would be it, yeah.

This is what happens when you start off basing a check of something that doesn't make use of the %s addon, then change your mind afterward. :V

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

I'm using codeblocks (bc free) and I couldn't get by at all without it's ability to tell me where something is declared and/or implemented. What are you using?

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

I seriously need to install MSYS2 to do compile testing. Usually it's more Notepad++ along with trial and error. And even then I only recently went from doing source code edits with plain Notepad or Wordpad. >.<

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

Oh man, I can't recommend something like codeblocks enough then. You can easily search the entire source code for things, hovering over a function name will tell you where it came from, and you can right click and find declaration/implementation and it takes you there. It took a little effort but I got it to compile for me too. What OS are you using, maybe I can assist you with getting compiling working. At the least I can send you what I used to make it happen.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

Ah. Never tried using Codeblocks for editing, I'd only thought to use it for compiling. Er, trying to at least. >.>

And hopefully it'll work alright, just kept forgetting to try it when I had the chance. Mostly because the few times I go poking around in the source code, I don't find out whether it's faster and more efficient to compile-test until after I've thrown up half a dozen failed commits. ._.

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

If I compile in curses it only takes about 10 minutes. Curses is good enough for most testing.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2016

Hmm. Never tried that. And predictably I yet again hit glorious inconsistencies to compiling.md's guide to using MSYS2. It starts with it saying one of the updates the guide says to grab is outdated and thus skipped, then pretty much unravels from there. >.>

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2016

I sent you a link on the forums. It has mingw64 as the compiler. That guide is a clusterfuck. After you get the libraries installed, I had to change the Project -> Build Options -> linker settings tab to include the right libraries. Also make sure it's using the compiler you want, it defaults to some kind of default craziness.
This is my linker settings tab, I had to add several of those libraries: http://puu.sh/mksKj/477674d812.png

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 17, 2016

Just...augh. I suck at actually handling the source, I suck at compile-testing, and so far all I've done is commit fuckups and get griped at for going the lazy route and using Jently to test this shit.

I'm starting to think I should just stop bothering with source-editing PRs entirely, if it's just going to be another string of stupid mistakes.

Chaosvolt
Consistency fix for best_shield
And here we go again...
@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Jan 18, 2016

I'm starting to think I should just stop bothering with source-editing PRs entirely, if it's just going to be another string of stupid mistakes.

There's nothing wrong with making a mistake so long as you learn from it.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Trying to. And failing, it seems. ._.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2016

Setting up a compiler helps a lot with learning because with a compiler you can just throw bad code, correct it a bit and repeat until it stops being so bad that it doesn't compile.

I checked the tutorial and I think the dead link you were talking about is now replaced by a proper one. At least all the links I checked were actually working.
The tutorial isn't perfect because it has extra steps (who needs tiles anyway?), but it should be working now.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Hmm. I should try it again, yes. Not sure which one to go with, since so far it's been three different compilers tried and just as many failures.

Still...sorry. Just got kinda set off by that. I know it's been nothing but fuckups on my end, and you don't really have time to constantly correct my mistakes. But when the first response to a question about what I'm doing wrong is to berate me for not being able to compile-test it, and call it "lack of respect" as if I'm intentionally trying to make things as much hassle as possible, is just...disheartening.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2016

But when the first response to a question about what I'm doing wrong is to berate me for not being able to compile-test it

It wasn't the first response.
First I wrote what has to be done. Then I wrote a lot of the code for you.
Then I explained what and where is wrong. Maybe not directly, but that's because you should be able to figure some things out on your own by reading compiler's complaints.

Only then I added a note about you not compiling things.

as if I'm intentionally trying to make things as much hassle as possible

Not intentionally, but it looks like you're trying to skip a part of the development because it's annoying to set up, hoping that there is a way around it.

This causes problems for everyone. Including you, because you can't compile it fast AND because even if someone tested it, it would most likely end up having bugs you'd have to fix before merging.

If you have problems with installing the compiler, ask. I bet you could even get someone on IRC to explain the entire tutorial step by step.
But don't ask how to do stuff after skipping setting up the environment. The best answer to that is "go back and set up the environment".

I'm not going to close the PR or anything like that, but criticism is a part of the review.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Sorry. Trying at the moment to follow the instructions for compiling (again), will likely get on the IRC once I'm done with the initial first steps.

And so far, I've found no evidence that compile-testing will actually be easier and faster than the way I've been doing it for, what, nearly a year by now (correction, about 10 months)? Even now I'm seeing a big potential issue in compiling: the need to add the required addons to the actual source directory.

My options are either to copy the entire source folder (which takes about as long as you'd expect 1.45 GB to) so that Github won't be trying to commit the added stuff to the pull request, or I'd have to move the needed files into it, then remember to move them elsewhere before commiting.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

And then there's the other issue. Given all the jokes I crack about the source code melting when I so much as look at it, it should be apparent that I inevitably tend to make some sort of mistake, and wind up at a complete loss for what I'm even doing wrong. If it's committed to the pull request, at least then others can look at it and find the predictable amateur fuckup that I completely failed to notice.

If I'm instead compile-testing it and not committing it, others can't actually look at my work, meaning I have to go and ask for help, then provide code excerpts of what all I'm doing when I eventually hit a problem and have no idea what I'm doing.

And even then, there's the possibility that when I post what I'm doing, I might miss important information in other files that might be the cause of the problem. And if I'm at the point where I don't know WHERE the problem is, it's rather obviously harder to provide those code excerpts, specifically excerpts that would actually reveal what part I fucked up.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2016

Committing unfinished PRs is fine (as long as you keep the [WiP] in title).
Do what you can, commit when you can't (or when you could and did).

I'm seeing a big potential issue in compiling: the need to add the required addons to the actual source directory.

If you compile without tiles, sound and localization, you don't need those.
Instead of make TILES=1 NATIVE=win32 LOCALIZE=1 it's make NATIVE=win32 LOCALIZE=0
You can add RELEASE=1 for faster compilation (but less helpful messages on crash).

The second windows tutorial is more complicated, but it doesn't meddle with source directory.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Hmm. Will be trying to get it set up then. For now though, I'm still at a loss for what to do now.

Just...sorry about this.

EDIT: Compiling in a nutshell:

fuck

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2016

http://stackoverflow.com/questions/29450016/o1-2-3-with-std-c1y-11-98-if-cmath-is-included-im-getting-error-hypo
However...this isn't an error I get when compiling which leads me to think you might be using an outdated mingw. What version do you have?

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

However...this isn't an error I get when compiling which leads me to think you might be using an outdated mingw. What version do you have?

The one linked to by COMPILING.md. >_>

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2016

Try the tutorial under "Rough guide to building with only MSYS2".
It is newer and thus less likely to cause any problems.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Well, time to repeat the mistakes I made when I tried that last time, I guess.

EDIT:

pacman --needed -Sy bash pacman pacman-mirrors msys2-runtime

I'm assuming that's supposed to be "bash pacman --needed -Sy bash pacman pacman-mirrors msys2-runtime" instead?

EDIT 2: Apparently Github format ate the "bash" and new line in that quote.

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2016

I'm a rebel and I always get the newest version of everything, it very very seldom is the wrong thing to do.

AngbandTK is an old ass game and I shouldn't have been surprised it wouldn't work with the brand new versions of it's requirements...

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Hmm. Straight-up copying all 3 lines into the command does jack shit.

@Malkeus

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2016

My ancient talisman protects me from your druidic chants!
Malkeus gestures frantically with an old eyball on a string. It menaces with spikes of cuteness! Carvings depict a hexecution of beetles in human skin.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Druidic chants? Something something tekeli-li. :V

Well, I'll see about compiling later on, if I get any farther in the attempt,

For now I'm still at that part where I'm stumped and would need someone more competent to figure out what I fucked up. >.<

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Jan 20, 2016

I can just upload somewhere my entire codeblocks folder with my working settings. Not sure if it'd wrote something in the registry, but you can try to download and use it nevertheless.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 20, 2016

That might help, unsure if it writes registry or appdata files, but is worth a try. ^^"

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Jan 20, 2016

@chaosvolt https://drive.google.com/file/d/0BwEAK44XlpItYXhTbGtKQ0s0eTQ/view?usp=sharing
I made some special version of source code for CodeBlocks to work properly - source code from github didn't worked for me. So if you want to try my version - just say, I'll upload it too.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 20, 2016

Ah, will look at it once I'm able to get back on the laptop, as will be out soon. Thank you.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2016

Well shit. Now merge conflicts. Greeeat.

return block_bonus;
}

std::string player::best_shield() const

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jan 22, 2016

Contributor

This function doesn't return a string. String is text. A reference to an item isn't a string of text, it's item &.

item &player::best_shield()
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2016

Ack. Is that all this needed to work? Scheisse.

Well...I'm actually already working on splitting this PR up. >.<

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2016

Closing this now that the important parts have been split into separate PRs.

@chaosvolt chaosvolt closed this Jan 25, 2016

@chaosvolt chaosvolt deleted the chaosvolt:shield-codening branch Jan 25, 2016

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