Implemented blip opcodes #436
Conversation
| data.type = BlipData::Instance; | ||
| data.colour = 6; // @todo 4 in Vice City | ||
| case BlipData::Coord: | ||
| data.colour = 5; |
There was a problem hiding this comment.
nit: The Vice City comments were usefull in my opinion.
//Edit: I just realized there are some comments about this below. I still don't understand this change, so maybe this comment can be ignored.
There was a problem hiding this comment.
Ugh, it was a long time ago, so I don't really remember what was the purpose of this change. BUT I think there was missing bilp type. Although, these values are from original game.
There was a problem hiding this comment.
It seems a function was added and the "deleted" values have been revised and git has, instead of being easy-to-read chosen to be efficient and reuse as many lines as possible. Looks a mess, but if one reads more code it stands clear that nothing was really lost, just re-organized. :)
| @arg arg2 | ||
| @arg arg3 | ||
| @arg blip Blip | ||
| opcode 0162 / 354 |
There was a problem hiding this comment.
The decimal is very confusing here
| } | ||
|
|
||
| /** | ||
| @brief %4d% = create_marker_above_actor %1d% color %2d% visibility %3d% | ||
| @brief ADD_BLIP_FOR_CHAR_OLD pedHandle colour display outblipHandle |
There was a problem hiding this comment.
Is this change really useful? The previous comment has been auto-generated. This change is only fine if we did this renaming for other functions too.
However, I'm also worried this might even be the R* function name, which we tried to avoid for legal reasons.
There was a problem hiding this comment.
In fact, it is original R* function name.
| @arg pedHandle Ped | ||
| @arg colour Blip colour | ||
| @arg display Blip display mode | ||
| @arg outblipHandle Created blip |
There was a problem hiding this comment.
The @arg was previously easily parsable by a computer. Now it's not. Check other instances and how we handled this in the past.
There was a problem hiding this comment.
I'm not sure I follow.
Was the name "argX" (where X is a number) easier to parse, or what is the actual difference?
There was a problem hiding this comment.
Take my commnt as an example on how not to write a review comment. I don't know what I meant myself.
But I think it has to do with the actual argument being called blip when this one refers to outblipHandle
| /** | ||
| @brief ADD_BLIP_FOR_OBJECT_OLD objectHandle colour display outblipHandle | ||
|
|
||
| opcode 0163 / 355 |
There was a problem hiding this comment.
For this comment block, see other instance above.
Also take care of other instances which follow.
| @@ -11717,8 +11716,8 @@ void opcode_03dc(const ScriptArguments& args, const ScriptPickup pickup, ScriptB | |||
| @arg arg2 | |||
| @arg blip | |||
| */ | |||
| void opcode_03dd(const ScriptArguments& args, const ScriptPickup pickup, const ScriptRadarSprite arg2, ScriptBlip& blip) { | |||
| auto data = script::createObjectBlipSprite(args, pickup, arg2); | |||
| void opcode_03dd(const ScriptArguments& args, const ScriptPickup pickup, const ScriptRadarSprite blipSprite, ScriptBlip& blip) { | |||
There was a problem hiding this comment.
Argument renamed but unchanged in comment. Please review other instances too.
|
Great input @JayFoxRox and @husho. Thanks! |
Renamed some functions and updated comments accordingly.
|
Comments have been addressed and taken care of. |
As written by @husho in #409