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

Fix table used as int #3604

Closed
wants to merge 3 commits into from
Closed

Conversation

linonetwo
Copy link

@linonetwo linonetwo commented Nov 4, 2024

fixes #3603

I think some items may have buggy value in config, while I find the config in this repo is good

so maybe bug comes from some other FU compat mod, that is outdated. But have guard in the code is always a good option. At least this make gun shootable again.

@edwardspec
Copy link
Contributor

edwardspec commented Nov 4, 2024

"magazineSize" : [5,8],

Intended behavior was to randomly choose a number between 5 and 8.
This is how it works when other parameters (such as damage) are set to an array of two numbers.

These are randomly generated weapons, so some of their parameters are randomized.

@linonetwo
Copy link
Author

linonetwo commented Nov 5, 2024

Hi, do you mean this is the Intended behavior in FU, but gunfire.lua forget to adapt to this usage?

Since the array [5, 8] in config is indeed interpreted as a table in Lua, which caused the original error because arithmetic operations can’t be directly applied to tables.

I can update PR to randomly pick a value in range at runtime here (So magazineSize is random for each shoot??). But I guess, the array [5, 8] in config should not be use directly here, it should generate an item data with a int instead of array. And we get the item data instead of config here. I'm new to modding so I'm not sure here.

@linonetwo
Copy link
Author

I think this PR is correct, I just add a guard to parse the int, if other mod copy the array to instance instead of generate an int before copy, this will fix it.

@edwardspec
Copy link
Contributor

edwardspec commented Nov 5, 2024

Currently this PR is incorrect, because it adds incorrect handling of magazineSize being a table:

tonumber(status.stat("magazineSize")) or 0

Situation: "magazineSize" : [5,8],
Expected behavior: magazineSize should be set to random value between 5 and 8.
Current behavior of your code: value of magazineSize is considered incorrect and set to default value of 0.

@edwardspec
Copy link
Contributor

edwardspec commented Nov 5, 2024

Are you sure you don't have any incompatible mods that overwrite items/buildscripts/buildweapon.lua?

Because I see correct handling of magazineSize being a table in the weapon's buildscript:

local newMagSize = ((parameters.isAmmoBased == 1) and configParameter("magazineSize",0)) or 0
if (parameters.isAmmoBased == 1) then
if type(newMagSize) == "table" then
if type(newMagSize[1]) == "number" and type(newMagSize[2]) == "number" then
newMagSize = math.random(math.min(newMagSize[1],newMagSize[2]),math.max(newMagSize[1],newMagSize[2]))

config.magazineSize = newMagSize

This code already replaces the table with an integer when the weapon is created.

@linonetwo
Copy link
Author

linonetwo commented Nov 5, 2024

Thank you for explaining it. I understand, I update the code to handle randomlization at runtime, while it should have done in weapon creation time.

The solution at least make the weapon useable. While it may change after restart game

图片
图片

Are you sure you don't have any incompatible mods that overwrite items/buildscripts/buildweapon.lua?

Seems it is impossible to know this in Starbound, I can't log who overwrites it.

But from the screenshot, looks like sometimes it has localization and sometimes not, maybe localization performs some bad patch.

@linonetwo
Copy link
Author

Some poor guys 5 years ago

So maybe it is weapon stats...Anyway I hope we can give this an end.

@sayterdarkwynd
Copy link
Owner

sayterdarkwynd commented Nov 5, 2024

Im not interested in nonsense bugs from 5 years ago that exist because people didn't bother to read our mod front page, which clearly stated that WeaponStats was not compatible in the first place.

Do you have Weapon Stats installed? If so, that is your issue, not FU. It is not compatible with our mod. No amount of adjusting the lua script will matter since Weapon Stats overwrites it. That, or you have a translation mod installed for FU, which is -also- not compatible.

I assume conflicts are the issue here, as the weapon ammo code has worked fine for years without incident.

Please confirm your mod selection with a log. I will not be approving this PR until this is done. There is also an error in the lua code you submitted.

@linonetwo linonetwo marked this pull request as draft November 5, 2024 13:36
@linonetwo linonetwo closed this Nov 5, 2024
@linonetwo
Copy link
Author

linonetwo commented Nov 5, 2024

Sorry for wasting your time, it was "Weapon Retrofitting Station - Breaks weapons" in the list.
And "New Weapon Type - Multi-turrets!" that is not yet in the list.

It says it is compatible in its page, but is not.

It may overwrite the buildweapon.lua, but not gunfire.lua, because my first commit on gunfire.lua makes it work.

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.

attempt to perform arithmetic on a table value
3 participants