Handling unexpected command *PART_ADAPTIVE_FAILURE #142

AliHaider93 · 2024-06-06T21:34:02Z

This code is written to handle unexpected command *PART_ADAPTIVE_FAILURE by adding it as an attribute to the region.

drossberg · 2024-06-07T17:25:48Z

src/conv/k-g/k-g.cpp

@@ -122,6 +122,8 @@ int main
 				bot.addTriangle(point1, point3, point4);
 			    }
 			}
+
+			    regions.setAttributes(partName,(it->second).attributes);


This has to be on the same level as the closing bracket, i.e. 4 spaces back.

src/conv/k-g/k_parser.cpp

drossberg · 2024-06-07T17:29:49Z

src/conv/k-g/k_parser.cpp

+			}
+			else {
+			    std::cout << "Attributes should come in pairs in k-file" << fileName << "\n";
+			    break;


The "break" is not needed here. The next one will do it for all.

drossberg · 2024-06-07T17:40:42Z

src/conv/k-g/region_list.cpp

+	bu_avs_add(avs, it->first.c_str(), it->second.c_str());
+    }
+
+    bu_avs_print(avs, name);


Maybe for debugging purpose, but I wouldn't leave it in the final code.

drossberg · 2024-06-07T17:43:52Z

src/conv/k-g/region_list.cpp

+    dbip = wdbp->dbip;
+    dp = db_lookup(dbip, name, 0);
+
+    rt_db_get_internal(&region_internal, dp, dbip, bn_mat_identity, &rt_uniresource);


Why bn_mat_identity and not NULL?

actually I didn't check what it means, now that I checked we are not applying matrix transform on the object and the correct way is to put NULL.

src/conv/k-g/region_list.cpp

drossberg · 2024-06-07T17:48:42Z

src/conv/k-g/region_list.cpp

+    struct rt_db_internal     region_internal;
+    struct directory*         dp;
+    struct db_i*              dbip;
+    bu_attribute_value_set*   avs;


This is very much C-style. In C++, you can declare the variables where you need them: declaration = initialization

you are right. I will modify it

drossberg · 2024-06-07T17:52:04Z

src/conv/k-g/region_list.h


 class RegionList {
 public:
    RegionList(void);

    Bot& addRegion(const std::string& name);

-    Bot& getBot(const std::string& name);
+    Bot&                                getBot(const std::string& name);


Why so many spaces?

because there was the function const std::map<std::string, std::string> getAttributes() under it and I wanted to kep them next to each other. then we deleted getAttributes() and I forgot to bring it back :)

AliHaider93 · 2024-06-08T09:05:35Z

src/conv/k-g/region_list.cpp


-    rt_db_get_internal(&region_internal, dp, dbip, bn_mat_identity, &rt_uniresource);
-    avs = &region_internal.idb_avs;
+    rt_db_get_internal(&region_internal, dp, dbip, NULL, &rt_uniresource);


do you think that it's neccessary to check the id returend by
rt_db_get_internal()==ID_COMBINATION?

That's a good point. However, the ID is not so important, because writeAttributes() could write the attributes to any database object. But, you should check if the command was successful, i.e. the return value is >= 0.

Same for dp?

they both need testing. I modified the code. please let me know if you have any notes

drossberg · 2024-06-09T12:34:06Z

src/conv/k-g/region_list.cpp

+	return;
+    }
+
+    if (rt_db_get_internal(&region_internal, dp, dbip, NULL, &rt_uniresource) <= 0) {


The documentation says that <0 means error, i.e. 0 would be okay too. In addition, even in case of error, region_internal has to be freed. Therefore, why not

if (rt_db_get_internal() >= 0) { // write the attributes // update database } else // error message // free region_internal

I did see in the code that most of the checks where rt_db_get_internal()<0, I think I should read more the documentaion is it the hacking guid you are talking about?. you are right the check you did is better I will modify.

drossberg · 2024-06-09T13:26:57Z

src/conv/k-g/region_list.cpp

+	return;
+    }
+
+    if (rt_db_get_internal(&region_internal, dp, dbip, NULL, &rt_uniresource) > 0) {


>= 0?
BTW, the documentation I referred to where the comments in the header file include/rt/db_internal.h.

Hmm, it should be in the Doxygen documentation https://brlcad.org/docs/api/ (could be a bit outdated)
https://brlcad.org/docs/api/d2/de5/db__internal_8h.html#a5fabf39a9c6246df52172a92c2a2b221

This are the kind of references you can find only if you know that they exist 😈

I am really sorry it's >=0. I was distracted

Handling unexpected command *PART_ADAPTIVE_FAILURE

468a80c

drossberg reviewed Jun 7, 2024

View reviewed changes

src/conv/k-g/k_parser.cpp Show resolved Hide resolved

drossberg reviewed Jun 7, 2024

View reviewed changes

src/conv/k-g/region_list.cpp Show resolved Hide resolved

drossberg reviewed Jun 7, 2024

View reviewed changes

update pull request

e0d4b6d

AliHaider93 commented Jun 8, 2024

View reviewed changes

AliHaider93 added 2 commits June 8, 2024 22:12

test rt_db_get_internal()

d85e449

test directory

92f004b

drossberg reviewed Jun 9, 2024

View reviewed changes

test rt_db_get_internal updated

01cfc8b

drossberg reviewed Jun 9, 2024

View reviewed changes

bigger or equal

ac794e1

drossberg merged commit 16f1fc3 into BRL-CAD:devel_k-g Jun 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handling unexpected command *PART_ADAPTIVE_FAILURE #142

Handling unexpected command *PART_ADAPTIVE_FAILURE #142

AliHaider93 commented Jun 6, 2024

drossberg Jun 7, 2024

drossberg Jun 7, 2024

drossberg Jun 7, 2024

drossberg Jun 7, 2024

AliHaider93 Jun 7, 2024

drossberg Jun 7, 2024

AliHaider93 Jun 7, 2024

drossberg Jun 7, 2024

AliHaider93 Jun 7, 2024

AliHaider93 Jun 8, 2024

drossberg Jun 8, 2024 •

edited

Loading

AliHaider93 Jun 8, 2024

drossberg Jun 9, 2024

AliHaider93 Jun 9, 2024

drossberg Jun 9, 2024

AliHaider93 Jun 9, 2024

Handling unexpected command *PART_ADAPTIVE_FAILURE #142

Handling unexpected command *PART_ADAPTIVE_FAILURE #142

Conversation

AliHaider93 commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drossberg Jun 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drossberg Jun 8, 2024 •

edited

Loading