-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Need a way to add non character variables (globals) #55155
Comments
Search for |
I see that "effect": [ { "u_location_variable": { "global_val": "NAME_OF_VAR" } } ] adds the variable, but it tracks location and seems to be designed for tracking world map changes based on its usage in the code. I'm looking for a similar solution to the basic u/npc_add_var and u/npc_has_var. Specifically has_var doesn't seem to have any support. I could do a compare_int to solve this it seems but that seems quite bloated for something as simple as global_has_var: "IS_THIS_NPC_DEAD" On the C++ side instead of the json side I wrote the only global var unit test so that isn't much help either 😆 |
All you need to do is use arithmetic and set a global var equal to 1. Take a look at cause_portal_storm |
Would there be value in me adding global_add_var and global_has_var for consistency? |
I don't know. I tend to want to get rid of the existing u_add_var, u_has_var, style stuff to force people to use the new more powerful arithmetic/compare_int functions and get rid of the confusion and make things more consistent. But I have my head way too buried in this stuff to know what's more helpful to other people. Which is better? To have a bunch of different functions that overlap functionality but some of which are a little simpler. Or only a few functions that don't overlap but can be a little verbose for simple commands? |
Moving towards the robust format is fine I didn't realize that was a goal. However based on:
is the plan to phase out string variable values because they are very useful for storing specific outcomes. |
So to be clear these are all my personal thoughts, no one else has commented on these issues as far as I know and even I am conflicted about them. So don't feel constrained by them at all, if you think adding global_has_var is a good idea and I'm being silly then I probably am and go for it.
This is a good question that's making me think. Honestly my forays into the json have revealed very few places where string variables are used as more than yes or no. And the few that are could be made numbers easily. That said its kind of shoving a square peg in a round hole and you are right that keeping around strings is generally good. One way to handle this would be to add the ability to use arithmetic/compare_int to handle string values but that might be overloading those methods too heavily. I'm not sure what the best answer is, there are plans(#53625) to potentially add more types of variables and the fewer of these methods we have floating around the easier that is but its not a huge deal. My main pet peeve is having too many different variable access syntaxes but maybe that's a trivial concern. Overall I might just be a stick in the mud here that's looked at this stuff for way too long. If it's easier for less experienced users to have global_has_var and the like functions than please add them and ignore my style nitpicking. |
yeah I'm super fine to go to one standard I think that is a good idea that's why I asked before just coding. This really is your chunk of the code so I wanted to make sure it was getting done the way you envision. If compare_int could be overloaded, generalized to compare_val or a separate compare_str added that would I think be good. As an example of where the strings could be useful here is {
"id": "TALK_GUNSMITH_CONVERT_TO_TYPE",
"type": "talk_topic",
"dynamic_line": {
"u_compare_var": "gunsmith_ammo_from",
"type": "number",
"context": "artisans",
"op": "==",
"value": 556,
"yes": "Sure, 5.56x45 to what?",
"no": {
"u_compare_var": "gunsmith_ammo_from",
"type": "number",
"context": "artisans",
"op": "==",
"value": 76251,
"yes": "Sure, 7.62x51 to what?",
"no": {
"u_compare_var": "gunsmith_ammo_from",
"type": "number",
"context": "artisans",
"op": "==",
"value": 76239,
"yes": "Sure, 7.62x39 to what?",
"no": {
"u_compare_var": "gunsmith_ammo_from",
"type": "number",
"context": "artisans",
"op": "==",
"value": 545,
"yes": "Sure, 5.45x39 to what?",
"no": {
"u_compare_var": "gunsmith_ammo_from",
"type": "number",
"context": "artisans",
"op": "==",
"value": 300,
"yes": "Sure, 300 BLK to what?",
"no": "If you are seeing this I am bugged and confused. You should report this fact on the GitHub."
}
}
}
}
}, in this case if I could have done string compares other places in the code gunsmith_ammo_from could have been the full cartridge type like: "5.45x39" and it could have just been embedded in the dialogue line instead of a tree of conditionals. To be clear I'm gonna just use 1 for the global "is the NPC dead" right now but I don't think string functionality should go away or be phased out. It's a great way of clearly tracking state for quest chains, values to embed in dialogue and some other things. |
A complicating question related to, but not on the topic: If the game has generated multiple copies of the artisans and you use a single global variable to track if an artisan is dead, wouldn't that affect all instances? |
Is your feature request related to a problem? Please describe.
Working on #55002 I want to make it so that if the NPC shopkeeper dies their shop stops refreshing. However ON_DEATH npc EOCs just let me access the dead NPC and the Killer. If the player is neither of these I can't keep track of the fact they are dead for the EOC to refresh their loot.
Solution you would like.
u_add_global_var or something similar. It's not clear how globals work currently in the JSON so I want to get a better understanding before trying to solve the problem.
Describe alternatives you have considered.
No response
Additional context
@Ramza13 if you have any thoughts on this, I'm happy to add it just don't know what the best way to do it is.
The text was updated successfully, but these errors were encountered: