Skip to content

Conversation

CrosRoad95
Copy link

@CrosRoad95 CrosRoad95 commented Nov 25, 2017

xmlLoadFile print error in console when somethink wrong with xml
Now you dont know what's wrong with your file, now you know!

example script lua

a=xmlLoadFile("wrong.xml")
iprint(a)

wrong.xml

<xml>
  asdf
  <b> </b>

  <a> </b>
</xml>

e5vrmfwbsukven-zyxksoq

@4O4
Copy link
Contributor

4O4 commented Nov 25, 2017

Someone give that man a cookie!

@AlexTMjugador
Copy link
Member

AlexTMjugador commented Nov 25, 2017

The idea behind this PR seems great. However, I'm a little concerned about backwards compatibility. For example, this code snippet would initialize a table with two fields instead of one, which can translate in unexpected behaviour of the script:

myTable = { xmlLoadFile("file.xml") }

Although I think that most scripters only take into account the first return value currently, I wouldn't take that for granted. IMHO, a 100% backward-compatible solution would be definitely better. This PR faced a similar problem which was solved by modifying the behaviour of the function according to min_mta_version. Maybe this could be improved the same way?

@Dezash
Copy link
Contributor

Dezash commented Nov 25, 2017

Better yet, instead of adding an extra field, just use LogWarning

@AlexTMjugador
Copy link
Member

@Dezash , that sounds nice to me 👍 . engineLoadDFF does that too when trying to load a bad DFF, if I remember correctly.

@CrosRoad95
Copy link
Author

CrosRoad95 commented Nov 26, 2017

i change that print error in debugscript insted of return argument.

@qaisjp qaisjp added pr:needs-testing enhancement New feature or request labels Nov 26, 2017
@CrosRoad95
Copy link
Author

CrosRoad95 commented Nov 27, 2017

xmlErrorTest.zip
Example resource which test errors.
7npfzjlxqlynwv7w8vib_g

@ccw808 ccw808 merged commit ee35900 into multitheftauto:master Nov 29, 2017
@ccw808
Copy link
Member

ccw808 commented Nov 29, 2017

Thanks!

ccw808 added a commit that referenced this pull request Nov 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants