Adds AddNoteFromText() to DXRStartMap. Uses it to give the player the hangar code note on a Hangar start.#1328
Adds AddNoteFromText() to DXRStartMap. Uses it to give the player the hangar code note on a Hangar start.#1328MQDuck wants to merge 1 commit intoDie4Ever:developfrom
Conversation
| } | ||
|
|
||
| #ifdef transcended | ||
| function DeusExNote AddNoteFromText(#var(PlayerPawn) player, bool bEmptyNotes, name textName, optional string textpackage, optional bool bRemoveBlankLines, optional bool bUserNote, optional bool bShowInLog, optional string strSource) |
There was a problem hiding this comment.
This function name should probably be "AddNoteFromTextTag", any time you're adding a note, it's probably "from text". Text Tag is a bit more specifically accurate. Likewise, the "textName" parameter should be "textTag", because that's what we actually call those.
"bRemoveBlankLines" seems like an odd option also - why would you want to not import the text from the text tag exactly as intended?
There was a problem hiding this comment.
This function name should probably be "AddNoteFromTextTag"
You're right. The current name isn't even accurate (it doesn't take a DeusExText as a parameter.
bRemoveBlankLines" seems like an odd option
Some datacubes have newlines removed when added to your notes. For instance, the hangar code datacube. Some don't.
Likewise, the "textName" parameter should be "textTag"
Yep, I like that a lot better. But I was following the naming convention of the vanilla code I was looking at.
There was a problem hiding this comment.
Are you sure those extra newlines aren't from tags that you aren't handling in the text?
<DC=255,255,255>
<P>Juan asked me for the hangar code, Decker, so it's 5914. Tell Young not to get his ass shot off -- you know how I feel about him. And I _will_ kill you if something happens.
<P>
<P>Take care,
<P>Erin
| if (parser == None) return None; | ||
|
|
||
| parser.OpenText(textName, textpackage); | ||
| while(parser.ProcessText()) { |
There was a problem hiding this comment.
You should likely look at how ProcessText is used in InformationDevices. I don't think there are any magic "blank lines" to be ignored, I think you're skipping tag information. This logic as-is likely would not function properly for text files that include references to the player name, use any formatting, etc. The logic here is missing all handling of the Tag property.
| parser = new(None) Class'DeusExTextParser'; | ||
| if (parser == None) return None; | ||
|
|
||
| parser.OpenText(textName, textpackage); |
There was a problem hiding this comment.
What happens if the textTag/textPackage combo doesn't actually exist? How does this handle that? OpenText appears to return a bool, which is presumably for success or failure. Look at other uses of this function.
| } | ||
|
|
||
| if (text == "") { | ||
| note = note $ "|n"; |
There was a problem hiding this comment.
Should this be a CR() instead of |n?
There was a problem hiding this comment.
Because that's the character used in other locations doing equivalent logic (It's a Carriage Return + Line Feed)
| note = note $ text; | ||
| } | ||
| } | ||
| CriticalDelete(parser); |
There was a problem hiding this comment.
The text should be closed before deleting the parser object. This may leave a dangling file reference or something. Not sure how bad this may be, exactly, but it should be done if only for correctness.
There was a problem hiding this comment.
I considered closing it but the vanilla code doesn't do that. I thought we weren't doing that either but I missed it. So yeah, I agree.
There was a problem hiding this comment.
See line 143 of InformationDevices from vanilla. I only mentioned it because Vanilla does it.
| @@ -716,6 +716,52 @@ function DeusExNote AddNoteFromConv(#var(PlayerPawn) player, bool bEmptyNotes, n | |||
| return None; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
As a high-level comment, I feel like this function should probably be in DXRActorsBase rather than in DXRStartMap
There was a problem hiding this comment.
I also agree with this, and it should be moved there along with all the rest of the Add*From* functions. I was just putting this one along with its cousins.
| if (bEmptyNotes == false) { | ||
| return None; | ||
| } | ||
| parser = new(None) Class'DeusExTextParser'; |
There was a problem hiding this comment.
Should all this logic to extract the string from the text be in this specific function, or should it be broken out to a different one that just returns the string from the text tag for you? Seems like that would be generally useful.
2 participants
No description provided.