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

Item ID typos and spelling errors for batch migration (add yours) #46289

Closed
int-ua opened this issue Dec 24, 2020 · 8 comments
Closed

Item ID typos and spelling errors for batch migration (add yours) #46289

int-ua opened this issue Dec 24, 2020 · 8 comments
Labels
[JSON] Changes (can be) made in JSON stale Closed for lack of activity, but still valid. <Suggestion / Discussion> Talk it out before implementing

Comments

@int-ua
Copy link
Contributor

int-ua commented Dec 24, 2020

Describe the bug

Some item IDs have errors in them. Items have a way to migrate them to new IDs via data/json/items/migration.json.

I suggest we collect all the IDs and migrate them in batch.

ID suggested replacement
balclava balaclava
lucern_hammer lucerne_hammer
bowhat bowlhat, hat_bowl, hat_bowler
porkpie hat_pork_pie
roller_blades rollerblades

Vehicle parts, if a migration is implemented:

VP ID suggested replacement
wood box wood_box
foldable wood box foldable_wood_box

Bionics

ID suggested replacement
mend_fault_bonic_salvaged_reset mend_fault_bionic_salvaged_reset

Additional context

#45878 #44460 #37967 #38073

@int-ua int-ua changed the title Item ID typos and errors for batch migration Item ID typos and errors for batch migration (add yours) Dec 24, 2020
@int-ua int-ua added <Suggestion / Discussion> Talk it out before implementing [JSON] Changes (can be) made in JSON labels Dec 24, 2020
@int-ua int-ua changed the title Item ID typos and errors for batch migration (add yours) Item ID typos and spelling errors for batch migration (add yours) Dec 24, 2020
@actual-nh
Copy link
Contributor

Consistent ax vs axe (I prefer the latter):

  • ax
  • copper_ax
  • hand_axe
  • makeshift_axe
  • primitive_axe
  • bowling_axe
  • throwing_axe
  • crash_axe
  • fire_ax
  • iceaxe
  • pickaxe

@hexagonrecursion
Copy link
Contributor

"wood box" and "folding wood box" are not items. They are vehicle parts. Is migration implemented for vehicle parts?

@Mom-Bun
Copy link
Contributor

Mom-Bun commented Jan 12, 2021

Why?
Players don't see the ID, this is merely dev surface, they don't need to be consistent or spelled correctly if you will only see it if you are debugging or something

@hexagonrecursion
Copy link
Contributor

tldr: developers, tileset makers and modders are people too. inconsistent ids cause confusion.

#46692 (comment)

I'm really opposed to this idea. This is churn for churn's sake, and probably breaks the effective DPS tests. There's no reason to change the item IDs for the various axes or the Lucerne Hammer - it doesn't matter if they're not spelled consistently or if lucern_hammer isn't spelled "correctly". It's an arbitrary token.

I understand your feelings but there is a reason: it will decrease possibility of misinterpretation and errors. Take a look at this commit: I-am-Erk/CDDA-Tilesets@24b9fae#diff-f9149e03f38263415f1e50300769b1b1ba54e84e990f7deae2014b2cb7ee50d4 Lucerne hammer had wrong ID in Ultica until I wrote the detection in the Tracker spreadsheet.

Another IDs I'd change are mag_* books as mag_pistol and mag_smg were misinterpreted as weapon magazines at least once: I-am-Erk/CDDA-Tilesets#251 (comment)

@actual-nh
Copy link
Contributor

In data/json/items/faults_bionic.json: "id": "mend_fault_bonic_salvaged_reset",

@int-ua int-ua mentioned this issue Jul 25, 2021
@int-ua
Copy link
Contributor Author

int-ua commented Dec 14, 2021

fema_entrance and FEMA_entrance are different OMTs https://discord.com/channels/598523535169945603/598614717799596055/920002564718723163
this pair alone blocks ability to rely exclusively on filenames for storing all OMT sprites as on filesystems that treat both cases as the same letter they are the same filename

@stale
Copy link

stale bot commented Apr 19, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

@stale stale bot added the stale Closed for lack of activity, but still valid. label Apr 19, 2022
@Night-Pryanik
Copy link
Contributor

Closing as stale, since stalebot can't do this by itself.

@Night-Pryanik Night-Pryanik closed this as not planned Won't fix, can't repro, duplicate, stale Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[JSON] Changes (can be) made in JSON stale Closed for lack of activity, but still valid. <Suggestion / Discussion> Talk it out before implementing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants