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

Cutting a web will now restore the previously wielded item. #517

Merged
merged 23 commits into from
Dec 20, 2018

Conversation

AquariusPower
Copy link
Contributor

@AquariusPower AquariusPower commented Dec 16, 2018

will still only spend one turn as this action is like a single slash on the web

and now you will not see/know what tool/weapon was used (TODO unless we message log it "You use %s to try to cut the nasty web."), there may have some complicated way to see the wielded being changed tho :>

(ready) bug fix for show items under (Attnam#369)
(ready) Fantasy name generator (Attnam#363)
@AquariusPower AquariusPower mentioned this pull request Dec 16, 2018
@jakwings
Copy link
Member

Ah, your editor introduced trailing spaces again...

@jakwings
Copy link
Member

btw should have a notice for lost weapons, otherwise it looks like the bug is not fixed.

@AquariusPower
Copy link
Contributor Author

AquariusPower commented Dec 17, 2018

btw should have a notice for lost weapons, otherwise it looks like the bug is not fixed.

But... it is already there

if(bLoseWeapon){
        rpd.itTool->RemoveFromSlot();
        rpd.itTool->MoveTo(rpd.lsqrPlaceAt->GetStack()); //TODO check if is not a WALL!!!
        ADD_MESSAGE("I lost my %s!",rpd.itTool->GetName(UNARTICLED).CStr());
        rpd.bAlreadyExplained=true;
}

Also the most important bug is actually the annoyance of having to swap weapons back to what it was, that breaks the game flow and makes players go away.

The important thing when coding, if you have not all the time of the world, is: priorities.
If we have no priorities we will spend a lot of time doing minor things other than crucial ones, we should spend time on minor things only after the crucial ones are fixed/working and after we have dealt with our own life matters.
So basically, to show the weapon when cutting a web (and so if it is lost could show an empty hand) makes no difference to the gameplay mechanics, therefore it is a minor thing. It can be considered a bug but certainly not a crucial one.

That's why I urge for ppl willing to help on coding.
More users, that are developers too, interested on the project was always my main goal, that's why I created the "console commands" and provided the debug messages project, but it being in BSD3 I didnt commit it here, but I think it can be committed like was done with some other libs right?
Such step, if someone could prepare, would help even more new devs to come by! So more hands to take care of tasks other devs have no time to do! ;)

I dont even have time to properly test anymore dammit... good I created the autoplay to give us back some of our time xD

@jakwings
Copy link
Member

But... it is already there

Just tested. It seems this problem only happened when I wielded two weapons and one of them is a dagger (on left or right hand).

It should be a good habit to remove unimportant trailing spaces, so when people fix others bugs they won't accidentally modify another unrelated files/lines, thus reducing merge conflicts. Or we should tell everyone not to strip trailing spaces, but if someone doesn't care, then again it's likely to produce merge conflicts. We'd better not leave the baggage to the future contribution. I know this is not a serious issue right at the moment but there is the potential.

The new BSD-3-clause license should be compatible with GPL: #388 (comment)

@AquariusPower
Copy link
Contributor Author

Just tested. It seems this problem only happened when I wielded two weapons and one of them is a dagger (on left or right hand).

as to it happen is random (and somewhat rare), it would be better to temporarily change the code to let it happen always, to be sure about the origin of the problem spending little time on it's simulation.

The new BSD-3-clause license should be compatible with GPL

so anyone can put dbgmsg files into a new folder like was done with xbrzlib,
prepare a CMakeLists file and it will be ready for new devs to use it,
I still wonder if there is a better/easierToUse debug log lib out there with more features and performance,
otherwise dbgmsg would still suffice for now.

I think I am going to need a good memory brain-implant one day, better save for it, thx xD

It should be a good habit to remove unimportant trailing spaces, so when people fix others bugs they won't accidentally modify another unrelated files/lines, thus reducing merge conflicts. Or we should tell everyone not to strip trailing spaces, but if someone doesn't care, then again it's likely to produce merge conflicts. We'd better not leave the baggage to the future contribution. I know this is not a serious issue right at the moment but there is the potential.

interesting, if I am not wrong, most of my last conflict solving were about things that could have been automatic out of it's lack of complexity, but I may be asking too much or may be github's merging tool configurations could be tweaked by us end users (I dont know if we even have access to such tho)?

I particularly think we should not annoy ppl willing to contribute with their spare time, ideas and energy, for free, to projects they love, because of minor things, or what will probably happen is that ppl will just create and distribute their forks (could that be what happened here in the past? if so cant we prevent it from happening again?) to escape from being bossed around like they are in their most probably oppressive and low paying jobs everywhere in this non utopic world, like in that they could be at parties having fun and many endless pleasures but they are sat behind their bright monitors, with a noisy fan nearby, focusedly coding to help us improve such marvels!
Aww dammit! Did I just woke up some potential contributor :o ?

But do not despair my friend! It will never be a serious issue if someone could spend some time on preparing a good C++ coding style template formatter to be run before every compilation, it would be amazing! :)

@jakwings
Copy link
Member

Man, please calm down. Other than you, nobody can set a deadline for your great work here.

If stripping trailing whitespace is not simple enough for you, fair, someone may do that for you in other pull requests.

Also, anyone can merge your code. This is only a conflict between you and me. My personal standard is just a little bit higher, at least before a true leader (boss?) appear.

My first priority is to play IVAN and to eliminate crashes. I'm sorry for reporting too many issues and not putting my hands directly on improving your code. Everyone's time is valuale.

Others please take care of this pr.

@AquariusPower
Copy link
Contributor Author

And I would never be able to properly fix festring neither make it work on Mac! much thx! :)

Don't understand me wrong also, I think it is great to have someone that likes to have all the code as perfect as possible, but I have a lot of difficulty doing that, one of the reasons I left my old job lol.

Also, I dont think the reports to improve this game should stop. Anyone who can spot things to be improved should do so! And anyone that have spare time could give us a hand also xD.

PS.: I was thinking, it seems we have quite a lot of open issues, I could even guess some already deprecated after so many changes, for ex. at the entt project the issues are closed after a few days apparently just to keep it clean, we could close all issues older than 6 months just because they are old, and if needed who created them just re-open with a new message and it will be refreshed. Just my opinion any way :)

@ryfactor
Copy link
Member

I'm not very good in these "grr" situations, I hope there's no hard feelings between you two.

I'm going to merge this PR anyhow, since it improves the user experience. The crash bug still needs fixing but that has been noted in #516

@AquariusPower I like your contributions so far, so keep it up. Upon reading your code I have found two ways your C++ coding could be improved:

  1. Avoid the use of #defines where a simple function would work fine. Preprocessor directives are ok, for example in whandler.cpp, but they came up in cmdcraft.cpp and they just made the code a bit harder for humans to read.

  2. The naming of your variables could be more descriptive. The legacy code we have does this to good effect. When I read lines like this one, I found myself needing to scroll up to figure out what the variables were referring to.

On the whole, your C++ is continually improving and I especially appreciate your emphasis on stable code, as evidenced by the debug tools you have written, and the way you alert others to bugs. It is true that the original devs coded many bugs, which we occasionally still find even today, and that did not stop them from releasing their work.

@iology you have rightly identified a significant gap in this project's organization, namely that we have no contribution guidelines. A coding style guide has been called for in the past, and this could be a good starting point.

@ryfactor ryfactor merged commit 902af15 into Attnam:master Dec 20, 2018
@jakwings
Copy link
Member

What I am angry about is: apparently, the time&energy spent on crashes, testing and code review is overlooked & who are willing to be responsible for hastily merging the OBVIOUSLY buggy code? Not me, and there is no deadline for either side so everyone has time to relax, make plans and do a better job instead of not trying to be a better and responsible developer.

I hope this pr will not caused any serious bug when I do not use those features. The coding style cannot solve the root problem.

Last, I just dislike the attitude, not anyone. I'm sure we can have more ways to cooperate more harmoniously if we are coding together in a cozy room, but, well... time to relax & congrats on Ivan055

@AquariusPower AquariusPower deleted the CutWebRestoWieldi branch December 21, 2018 00:24
@AquariusPower
Copy link
Contributor Author

Avoid the use of #defines where a simple function would work fine. Preprocessor directives are ok, for example in whandler.cpp, but they came up in cmdcraft.cpp and they just made the code a bit harder for humans to read.

agreed, I think a function to fill the recipes would be better indeed.

The naming of your variables could be more descriptive. The legacy code we have does this to good effect. When I read lines like this one, I found myself needing to scroll up to figure out what the variables were referring to.

h a character but of human type, something like humanoidCrafter would be better I guess :)

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.

3 participants