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

Craft Skill and other related things #595

Merged
merged 57 commits into from
Oct 26, 2021
Merged

Conversation

AquariusPower
Copy link
Contributor

@AquariusPower AquariusPower commented Apr 25, 2020

It has also a fix for crafting something that breaks and has no config at .dat files.
(cherry pick from #587)

AquariusPower and others added 30 commits May 24, 2018 16:22
(ready) bug fix for show items under (Attnam#369)
(ready) Fantasy name generator (Attnam#363)
@AquariusPower
Copy link
Contributor Author

this should receive an update I didnt mange yet to bring from #587
about a chance of gaining craft skill every craft turn on long lasting crafting tasks.

tailoring workbench and related graphics
other craft related improvements (cherry picking got overly complicated and time consuming so I just copied the whole related files (or merged parts of them using `Meld`) from the other branch)
@red-kangaroo
Copy link
Contributor

@AquariusPower One thing I noticed in #596 : When splitting rocks with no appropriate tool, the game tells you that you need a cutting tool, but you actually need a hammer.

}

virtual void fillInfo(){
init("build","a tailoring workbench");
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe change normal workbench to "crafting workbench" then, to make it clear in the F menu how are the two workbenches different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

may be "smithing bench" but I read it is more related to metal work, "crafting workbench" is more generic indeed and may be clearer

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, "smithing" is metal work and we already have an anvil for that. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Change made. :)

@red-kangaroo
Copy link
Contributor

Could we get reorganized skill overview? Right now, it says "Your experience in weapon categories"/"battle bonus", and then lists crafting. :)

There's crafting skill in the @ menu, and then crafting proficiency in F menu. Does crafting skill increase your crafting proficiency?

When crafting from ingots, they are instantly transformed to lumps if you cancel the crafting after selecting materials but before beginning the crafting action. They should only get lumped if you start crafting and then fail/stop.

What if you could craft any furniture, even if they don't do anything? Maybe the player wants to build a small house with some beds and an oven.

Spider silk from webs is a great touch. :)

@AquariusPower
Copy link
Contributor Author

@AquariusPower One thing I noticed in #596 : When splitting rocks with no appropriate tool, the game tells you that you need a cutting tool, but you actually need a hammer.

if I am not wrong, I initially thought and coded it like: to split a rock you would need a dagger and a hammer, so that you would aim the dagger on the rock and hit the hammer on the dagger's hilt. But the warn/user/help message may be imprecise!

@AquariusPower
Copy link
Contributor Author

AquariusPower commented May 30, 2020

Could we get reorganized skill overview? Right now, it says "Your experience in weapon categories"/"battle bonus", and then lists crafting. :)

The skill progress system was already implemented as weapon skills, it just took me some more time than usual to realize that xD

There's crafting skill in the @ menu, and then crafting proficiency in F menu. Does crafting skill increase your crafting proficiency?

in the F menu (that is a good and innocent menu, just meaning craFting xD), so it works like this:
You have the craft skill, that increases with each related action.
You have a crafting bonus based on your stats (mainly dexterity).
On that special menu, the crafting value shown is: CraftSkill+CraftBonus
Btw, craft bonus now begins in 0 if you have all stats in 10, what is an incentive to develop the craft skill. (I think there is a limit tho, the final value is never less than 1)

When crafting from ingots, they are instantly transformed to lumps if you cancel the crafting after selecting materials but before beginning the crafting action. They should only get lumped if you start crafting and then fail/stop.

Agreed! and the way it is now it is quite annoying/unfair...
But to let it work properly, it would require a quite more complex code (I guess),
or... I just thought it... we could implement an "UNDO" code, that would join the lump back into the ingot if the crafting action doesnt really begin.
Cutting the ingot (or any other crafting basic item) and creating the lump is a preparation to the crafting process but it should be undone if that isnt started!

Cool, I think an UndoCraftingPreparation() could be a good workaround!
May be, the new cut ingot and the new lump could be just deleted and the old one could be just restored, I dont know yet the best way to implement it tho.

What if you could craft any furniture, even if they don't do anything? Maybe the player wants to build a small house with some beds and an oven.

Nice! it would open a menu listing them all, may be the useful ones on the top.
The problem is the volume of materials required to build them. May be all "non useful" could have a default small value, may be 2000cm3,
or we could add a property on the .dat file related to furniture, something like: ToCraftVolumeRequired = 2000, that being default, but later we could specify each, and it would be better.

Spider silk from webs is a great touch. :)

Used in tailoring as sewing material, but we can craft gloves etc from it too, yep it is fun.
After that I avoid killing spiders to let them create webs I can collect. Btw, in my house irl, I get all these inoffensive spiders and put them on the plants to help fighting insects, inoffensive spiders are very cool! xD

- polymorphed also affects craft skill;
- fixed missing tailoring savegame data;
- fixed remaining turns calculation to work as originally intended;
- reworked fumble algorithm, generally it feels much better/faster (may be easier too) to use crafting now;
@AquariusPower AquariusPower changed the title Craft Skill Craft Skill and other related things Jun 15, 2020
@ryfactor
Copy link
Member

It compiles and runs on Windows. I haven't tested anything but it seems to work OK.
Can we merge this yet?

@ryfactor
Copy link
Member

I would like to merge this next. I think it is ok @red-kangaroo ?

Copy link
Contributor

@red-kangaroo red-kangaroo left a comment

Choose a reason for hiding this comment

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

Several tiny gripes I had fixed, otherwise seems good. :)

@@ -382,7 +382,12 @@ void game::InitScript()

truth game::IsQuestItem(item* it) //dont protect against null item* it may be a problem outside here.
{
return it->IsQuestItem();
return it->IsQuestItem() //TODO this call should suffice instead of this very function
Copy link
Contributor

Choose a reason for hiding this comment

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

These extra checks are redundant.

@ryfactor ryfactor merged commit a3e5865 into Attnam:master Oct 26, 2021
ryfactor pushed a commit that referenced this pull request Oct 26, 2021
…aft Skill PR #595) (#596)

* change light emitation (to less) based on object volume

* fixed minimum emitation and workaround/fix for other related bugs
still highly depends on crafting updated code tho

* light emit based on volume, reworked calc.
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