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

identify items / cursed items #160

Open
picobyte opened this issue Feb 17, 2020 · 7 comments
Open

identify items / cursed items #160

picobyte opened this issue Feb 17, 2020 · 7 comments
Milestone

Comments

@picobyte
Copy link
Contributor

I think it would be a nice to have to identify magic items, by a spell or in the city, which would allow for cursed items. Closed issue #89 seems related.

@Grokmoo Grokmoo added this to the 0.5.0 milestone Feb 21, 2020
@picobyte
Copy link
Contributor Author

The feature branch for this is:
https://github.com/picobyte/sulis/tree/identification

@Grokmoo
Copy link
Owner

Grokmoo commented Feb 24, 2020

Please discuss the implementation details before working on a feature like this.

What is "taxator" mean (I'm not familiar with that term) and what is it doing?

In my mind, this requires an identified flag on item_state and in item_save_state. This would allow the identified state to be set manually wherever an item is specified, such as in an area. In addition, in the loot list we would need some chance that items are unidentified. It would only make sense for items generated with certain adjectives. So, each item_adjective would need some sort of flag to indicate whether it can be generated as unidentified.

If an item in a loot list is generated with one or more such flags, then we could use the loot_list unidentified item chance.

At that point, we need UI support in sulis_view. We need to hide details for item_states that are unidentified. Identifying items could probably we worked into the merchant screen. A merchant could have a simple defined identification price or maybe it could have a defined fraction of the items' value?

Overall, this is quite a large feature.

@picobyte
Copy link
Contributor Author

picobyte commented Feb 24, 2020

Sorry, I thought taxator and to taxate were English terms (In dutch we have taxateur, it's from Latin), appraiser is probably the better translation: he/she who is able to recognize the item for its worth. In my patch I made it a separate person, not a merchant, but I guess we could also add it to the merchant view.

I've indeed added the indentified (oops, typo) on the item_state, but didn't add an item_save_state yet. I had identified default to false, but you're right, there should be a identified chance and particular items should have it.
I was thinking about allowing to use an item, without the bonuses, if any. In the case of a cursed item, the item removal could become, impossible until dispel or for a certain time, or there could be sites, people or conditions where one can remove the item. Cursed items could have effects as well.

A fraction of the item value (5-10%) was what I had in mind to calculate the price. I named it taxate_frac, but can rename it to appraise_frac. Here I adapted the inventoryWindow, but wasn't happy with it. Thought about maybe making a separate view, but you're right, the addition to the merchant may be easier. We could also make it possible for some merchants to appraise particular items only.

@picobyte
Copy link
Contributor Author

picobyte commented Feb 29, 2020

Since the identification will be something of the merchants view, I'll work in a clean branch. I have a running version with most of the framework, but not the merchants view, it seems to run, save & load though the identification status is not yet evident.

https://github.com/picobyte/sulis/tree/identification_try2

I've added the identified flag to the item_state as suggested. There was ItemState::from(id: &str) that I've let default to identified: true, otherwise I'm not sure what the best strategy would be, the same goes for ScriptItemKind.

In item_save_state it is better to have "unidentified" as ability, or every item for a pc (new game) requires an identified flag for loading, which seems redundant.

each item_adjective would need some sort of flag to indicate whether it can be generated as unidentified.

Can't we make unidentified an adjective itself? I was thinking about an 'unidentified' fraction. In gen_adjectives() there are two weights, compared against rolls, one for quality, one for bonuses. Could we use the unidentified as an adjective2 (bonus)?

data/item_adjectives/bonus_postfix_low/unidentified1.yml
data/item_adjectives/bonus_postfix_mid/unidentified2.yml
data/item_adjectives/bonus_postfix_high/unidentified3.yml

e.g.

id: unidentified2
name: Unidentified
name_postfix: " unidentified"
item_status_icon: gui/status_unidentified
value_modifier: 0.2
value_add: 0
bonus_modifier: 1.0
penalty_modifier: 1.0
attack_damage_modifier: 1.0
attack_bonus_modifier: 1.0
attack_penalty_modifier: 1.0
bonuses:
  - kind:
      unidentified: 0.6
attack_bonuses: {}

Others are the same, except the unidentified ratio. I've set up the status_unidentified icon to be placed at the end of the first row. Not yet added a icon in the sprite sheet, maybe a light grey question mark? alternatively we could draw an icon that covers the entire item with a semi-transparent overlay, but then this icon should be placed elsewhere in the sprite sheet.

@Grokmoo
Copy link
Owner

Grokmoo commented Mar 2, 2020

Since the identification will be something of the merchants view, I'll work in a clean branch. I have a running version with most of the framework, but not the merchants view, it seems to run, save & load though the identification status is not yet evident.

https://github.com/picobyte/sulis/tree/identification_try2

I've added the identified flag to the item_state as suggested. There was ItemState::from(id: &str) that I've let default to identified: true, otherwise I'm not sure what the best strategy would be, the same goes for ScriptItemKind.

Yes, default true sounds good.

In item_save_state it is better to have "unidentified" as ability, or every item for a pc (new game) requires an identified flag for loading, which seems redundant.

In item_save_state, we definitely want to use #[serde(default="identified_default"] on the identified field. Then separately, define: fn identified_default() -> bool { true }. Otherwise, as you say, you will need to specify it on every single ItemSaveState in all data files, and it will also break all old save game.

I'm not sure what you mean by "unidentified as ability".

each item_adjective would need some sort of flag to indicate whether it can be generated as unidentified.

Can't we make unidentified an adjective itself? I was thinking about an 'unidentified' fraction. In gen_adjectives() there are two weights, compared against rolls, one for quality, one for bonuses. Could we use the unidentified as an adjective2 (bonus)?

Just a minor note, adjective2 vs adjective1 are not treated any differently in the code.

I'm not sure if I like the approach of having Identified be an ItemAdjective. I see a lot of pluses, and a lot of minuses. I guess if you want to try this route you'll need to go down it and see where it leads. One concern is how will you specify which of the other adjectives are hidden and which aren't? Probably, we don't want to hide whether an item is item is "Fine" or "Exceptional" but do want to hide most other adjectives.

Actually as a general comment, hiding the bonuses may be quite difficult - as they will need to not show up on the character sheet in any way. I'm not sure if we want them to still apply in actual combat - probably yes.

data/item_adjectives/bonus_postfix_low/unidentified1.yml
data/item_adjectives/bonus_postfix_mid/unidentified2.yml
data/item_adjectives/bonus_postfix_high/unidentified3.yml

I don't understand why you have 3 different unidentified adjectives.

e.g.

id: unidentified2
name: Unidentified
name_postfix: " unidentified"

Unidentified should definitely come before the name in English, not after. If you use adjectives this will need to be special cased in any event as we will need to block out the display of (some of?) the other adjectives.

item_status_icon: gui/status_unidentified
value_modifier: 0.2
value_add: 0
bonus_modifier: 1.0
penalty_modifier: 1.0
attack_damage_modifier: 1.0
attack_bonus_modifier: 1.0
attack_penalty_modifier: 1.0
bonuses:

  • kind:
    unidentified: 0.6
    attack_bonuses: {}

Others are the same, except the unidentified ratio. I've set up the status_unidentified icon to be placed at the end of the first row. Not yet added a icon in the sprite sheet, maybe a light grey question mark? alternatively we could draw an icon that covers the entire item with a semi-transparent overlay, but then this icon should be placed elsewhere in the sprite sheet.

What does the unidentified ratio do? Why is it 0.6? I do not think unidentified should go in the bonus list as it is not a bonus and is not applied to the owning creature.

@picobyte
Copy link
Contributor Author

picobyte commented Mar 2, 2020

I'm not sure what you mean by "unidentified as ability".

Sorry, I meant adjective. struct ItemSaveState is where you suggest I add this, right?

In item_save_state, we definitely want to use #[serde(default="identified_default"] on the identified field. Then separately, define: fn identified_default() -> bool { true }.

Your solution should work as well, but I tried to avoid having to modify all ItemSaveStates by using an "unidentified" adjective, which defaults to not present == identified. Did I miss something?

One concern is how will you specify which of the other adjectives are hidden and which aren't? Probably, we don't want to hide whether an item is item is "Fine" or "Exceptional" but do want to hide most other adjectives.

Agreed, all except quality.

The way it's currently implemented, there seems just one roll per adjective resulting in a quality(1) and one bonus adjective(2). More work to do in descriptions, but would multiple bonuses, for adjective 2, be a feature?

Actually as a general comment, hiding the bonuses may be quite difficult - as they will need to not show up on the character sheet in any way.

I don't seem to obtain unidentified items in game yet, with my identified code. When I have that fixed, we'll see (or not - unidentified).

I'm not sure if we want [hidden bonuses] to still apply in actual combat - probably yes.

When wielded, we could allow the bonus to work, dependent on chance. If it does, then the item could become identified, maybe another chance. A tempting but unsafe kind of identification, once there are cursed items as well.

data/item_adjectives/bonus_postfix_low/unidentified1.yml
...
data/item_adjectives/bonus_postfix_high/unidentified3.yml

I don't understand why you have 3 different unidentified adjectives.

Initially, I thought I had to add the chance in the kind section, but I later realized I could simply just add the chance to the loot_list. I agree only one is needed and about the redundant unidentified ratio:

What does the unidentified ratio do? Why is it 0.6? I do not think unidentified should go in the bonus list as it is not a bonus and is not applied to the owning creature.

@picobyte
Copy link
Contributor Author

picobyte commented Mar 9, 2020

The identification changes are in pull request #175

Unidentified is no longer a bonus kind in the yaml. Unidentified is an adjective2. If unidentified is the first item's adjective2 roll, a second adjective 2 is selected (which is the hidden one). To this extent I added the generic function for selection from weighted choices. It is maybe a more thorough cleanup than strictly necessary, but some change was needed to implement the unidentified re-roll and could make multiple bonuses for weapons easier to implement, I think.

Concerning item status display, "Unidentified" is not even implemented as a prefix because the displayed name needs to act differently, hide bonuses if unidentified. The display name is concatenated in the itemState::get_name() function. Images on buttons are also hidden, as well as actor_state listings.

Identification can occur at the merchant. A first left-click on an unidentified item identifies the item, for 20% of the value, the second click sells the item for the remaining 80%.

I have some more changes, to implement a curse, but I think that can probably go in a new feature branch.

@Grokmoo Grokmoo modified the milestones: 0.6.0, Future Aug 19, 2020
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

No branches or pull requests

2 participants